CXF
  1. CXF
  2. CXF-3558

JaxWsProxyFactoryBean.create is not thread-safe

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4.1, 2.3.5
    • Component/s: JAX-WS Runtime
    • Labels:
      None
    • Estimated Complexity:
      Moderate

      Description

      While running JaxWsProxyFactoryBean.create concurrently I encountered the following exception:

      Caused by: java.util.ConcurrentModificationException
      	at java.util.LinkedList$ListItr.checkForComodification(LinkedList.java:761)
      	at java.util.LinkedList$ListItr.next(LinkedList.java:696)
      	at org.apache.cxf.service.factory.AbstractServiceFactoryBean.sendEvent(AbstractServiceFactoryBean.java:71)
      	at org.apache.cxf.service.factory.ReflectionServiceFactoryBean.create(ReflectionServiceFactoryBean.java:238)
      	at org.apache.cxf.jaxws.support.JaxWsServiceFactoryBean.create(JaxWsServiceFactoryBean.java:202)
      	at org.apache.cxf.frontend.AbstractWSDLBasedEndpointFactory.createEndpoint(AbstractWSDLBasedEndpointFactory.java:101)
      	at org.apache.cxf.frontend.ClientFactoryBean.create(ClientFactoryBean.java:90)
      	at org.apache.cxf.frontend.ClientProxyFactoryBean.create(ClientProxyFactoryBean.java:152)
      	at org.apache.cxf.jaxws.JaxWsProxyFactoryBean.create(JaxWsProxyFactoryBean.java:142)
      

      Although not documented , org.apache.cxf.service.factory.ReflectionServiceFactoryBean#create called somewhere during endpoint creation is synchronized which suggests whole create is thread-safe, but apparently it's not.

      I managed to discover source of this problem. I captured two threads: one (pool-1-thread-4) entered synchronized ReflectionServiceFactoryBean#create, the second one (pool-1-thread-2) didn't made there yet but instead it calls AbstractServiceFactoryBean.setBus outside of the synchronized block. This causes the org.apache.cxf.service.factory.AbstractServiceFactoryBean#listeners to be updated, making the former thread being susceptible to ConcurrentModificationException. Stack dumps attached.

      Please either fix concurrency issue (making listeners instance of java.util.concurrent.CopyOnWriteArrayList might not be enough though) or document that JaxWsProxyFactoryBean.create is not thread-safe.

      1. CXF-3558.txt
        3 kB
        Tomasz Nurkiewicz

        Activity

        Hide
        Willem Jiang added a comment -

        You don't need to create the proxy per thread. CXF client proxy is thread safe

        Show
        Willem Jiang added a comment - You don't need to create the proxy per thread. CXF client proxy is thread safe
        Hide
        Freeman Fang added a comment -

        Hi Tomasz,

        As Willem mentioned here, the proxy itself is actually thread safe already, do you really need create multiple proxies per thread, I can't image a necessary scenario for it, could you please clarify your requirement?
        Moreover, I don't think currently the way to create a proxy is thread safe by design, besides the create method, we also have lots of other variable for ClientProxyFactoryBean like username/password/properties/bus/features/databinding which can cause uncertain if you try to set those from different thread.

        So the correct way is create only one proxy and use it in different thread.

        Freeman

        Show
        Freeman Fang added a comment - Hi Tomasz, As Willem mentioned here, the proxy itself is actually thread safe already, do you really need create multiple proxies per thread, I can't image a necessary scenario for it, could you please clarify your requirement? Moreover, I don't think currently the way to create a proxy is thread safe by design, besides the create method, we also have lots of other variable for ClientProxyFactoryBean like username/password/properties/bus/features/databinding which can cause uncertain if you try to set those from different thread. So the correct way is create only one proxy and use it in different thread. Freeman
        Hide
        Tomasz Nurkiewicz added a comment -

        Thank you very much for your comment. I am perfectly aware client is thread-safe once created and this is the way I use it typically. But I started to play a bit with clustering/failover features in Apache CXF which state in their JavaDocs (LoadDistributorFeature and FailoverTargetSelector): Note that this feature changes the conduit on the fly and thus makes the Client not thread safe.

        So my solution was to pool the clients and create as many clients as necessary using just one factory (username/password etc. won't change once configured). And this is where the problem manifests - if two threads simultaneously try to create two CXF clients (since they shouldn't share the same one due to the load-balancing/failover restrictions mentioned above). I wrote some small article about it.

        Show
        Tomasz Nurkiewicz added a comment - Thank you very much for your comment. I am perfectly aware client is thread-safe once created and this is the way I use it typically. But I started to play a bit with clustering/failover features in Apache CXF which state in their JavaDocs ( LoadDistributorFeature and FailoverTargetSelector ): Note that this feature changes the conduit on the fly and thus makes the Client not thread safe. So my solution was to pool the clients and create as many clients as necessary using just one factory (username/password etc. won't change once configured). And this is where the problem manifests - if two threads simultaneously try to create two CXF clients (since they shouldn't share the same one due to the load-balancing/failover restrictions mentioned above). I wrote some small article about it.
        Hide
        Freeman Fang added a comment -

        Hi,

        Thanks for your clarification.
        I get your point, so if your factory won't change any properties once configured, we can synchronized ClientProxyFactoryBean create() method, as well as use ModCountCopyOnWriteArrayList for ListenerList, ensure that the proxy creation is thread safe.

        Freeman

        Show
        Freeman Fang added a comment - Hi, Thanks for your clarification. I get your point, so if your factory won't change any properties once configured, we can synchronized ClientProxyFactoryBean create() method, as well as use ModCountCopyOnWriteArrayList for ListenerList, ensure that the proxy creation is thread safe. Freeman
        Show
        Freeman Fang added a comment - commit fix http://svn.apache.org/viewvc?rev=1129592&view=rev for trunk http://svn.apache.org/viewvc?rev=1129594&view=rev for 2.3.x branch
        Hide
        Freeman Fang added a comment -

        Hi Tomasz,

        Btw, your blog about "Enabling load balancing and failover in Apache CXF" is really good, I've added the link to cxf resources and articles list.

        Thanks
        Freeman

        Show
        Freeman Fang added a comment - Hi Tomasz, Btw, your blog about "Enabling load balancing and failover in Apache CXF" is really good, I've added the link to cxf resources and articles list. Thanks Freeman

          People

          • Assignee:
            Freeman Fang
            Reporter:
            Tomasz Nurkiewicz
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development