DefaultHttpRegistry thread safety

classic Classic list List threaded Threaded
2 messages Options
jnk
Reply | Threaded
Open this post in threaded view
|

DefaultHttpRegistry thread safety

jnk
 Hi guys,
have you ever considered making DefaultHttpRegistry thread safe?

Currently DefaultHttpRegistry uses common HashSets for storing
HttpConsumers and CamelServlets. So when you are trying to register more
consumers in paralel it can lead to missing consumer in hashset due to its
nature. Another case is when you are registering consumers and servlets in
paralel. This case can lead to ConcurrentModificationException.


private final Set<HttpConsumer> consumers;
private final Set<CamelServlet> providers;

public DefaultHttpRegistry() {
   consumers = new HashSet<>();
   providers = new HashSet<>();
}

public void register(HttpConsumer consumer) {
   if (LOG.isDebugEnabled()) {
       LOG.debug("Registering consumer for path {} providers present: {}",
consumer.getPath(), providers.size());
   }
   consumers.add(consumer);
   for (CamelServlet provider : providers) {
       provider.connect(consumer);
   }
}

public void register(CamelServlet provider) {
   if (LOG.isDebugEnabled()) {
       LOG.debug("Registering CamelServlet with name {} consumers present:
{}", provider.getServletName(), consumers.size());
   }
   providers.add(provider);
   for (HttpConsumer consumer : consumers) {
       provider.connect(consumer);
   }
}


I know there's possibility to use own http registry in camel route starting
with servlet. But you have to register it to servlet-component, then
implement custom servlet because nits DefaultHttpRegistry in it's init
method. Seems to me like a lot of ceremony to have thread safe engine. Am I
missing something?

Thanks

Jiri
Reply | Threaded
Open this post in threaded view
|

Re: DefaultHttpRegistry thread safety

Claus Ibsen-2
Hi

Ah okay yeah it can help to make it thread-safe. Haven't had this
reported before, and I guess also Tomcat etc and others dont start
servlets in parallel. But you are surely welcome to create a JIRA and
work on a github PR with a fix

On Camel 2.x its only the 2.25.x branch that is active, and on 3.x is
its only master.

On Sun, Mar 1, 2020 at 10:30 AM Jiří Novák <[hidden email]> wrote:

>
>  Hi guys,
> have you ever considered making DefaultHttpRegistry thread safe?
>
> Currently DefaultHttpRegistry uses common HashSets for storing
> HttpConsumers and CamelServlets. So when you are trying to register more
> consumers in paralel it can lead to missing consumer in hashset due to its
> nature. Another case is when you are registering consumers and servlets in
> paralel. This case can lead to ConcurrentModificationException.
>
>
> private final Set<HttpConsumer> consumers;
> private final Set<CamelServlet> providers;
>
> public DefaultHttpRegistry() {
>    consumers = new HashSet<>();
>    providers = new HashSet<>();
> }
>
> public void register(HttpConsumer consumer) {
>    if (LOG.isDebugEnabled()) {
>        LOG.debug("Registering consumer for path {} providers present: {}",
> consumer.getPath(), providers.size());
>    }
>    consumers.add(consumer);
>    for (CamelServlet provider : providers) {
>        provider.connect(consumer);
>    }
> }
>
> public void register(CamelServlet provider) {
>    if (LOG.isDebugEnabled()) {
>        LOG.debug("Registering CamelServlet with name {} consumers present:
> {}", provider.getServletName(), consumers.size());
>    }
>    providers.add(provider);
>    for (HttpConsumer consumer : consumers) {
>        provider.connect(consumer);
>    }
> }
>
>
> I know there's possibility to use own http registry in camel route starting
> with servlet. But you have to register it to servlet-component, then
> implement custom servlet because nits DefaultHttpRegistry in it's init
> method. Seems to me like a lot of ceremony to have thread safe engine. Am I
> missing something?
>
> Thanks
>
> Jiri



--
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2