This repository was archived by the owner on Jan 27, 2021. It is now read-only.
Bugfix: Prevent deadlock in ComponentProcessorFactoryImpl #6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The class `ComponentProcessorFactoryImpl` holds an internal `ConcurrentMap`, that is being used to store component processors per source class. The `get` method uses a double-checked locking pattern to keep this map updated in a thread-safe manner, using locking with `synchronized` if the map does not already contain a map. If the map is not pre-filled, the `get` method will populate it into the map. However, the `synchronized` locking is done on the wrong object. This commit updates the double-checked locking so that the locking is done on the map being checked and updated, instead of locking on another field in the enclosing class. This prevents a situation where two threads could deadlock each other. The following is an excerpt of a jstack dump from one of our production Glassfish servers: ``` Found one Java-level deadlock: ============================= "__ejb-thread-pool1": waiting to lock monitor 0x00000000032a4518 (object 0x00000007bc75a9a0, a java.util.HashMap), which is held by "main" "main": waiting to lock monitor 0x00000000032a38b8 (object 0x00000007bd113780, a java.util.concurrent.ConcurrentHashMap), which is held by "__ejb-thread-pool1" Java stack information for the threads listed above: =================================================== "__ejb-thread-pool1": at com.sun.jersey.server.impl.application.WebApplicationImpl$ComponentProcessorFactoryImpl.get(WebApplicationImpl.java:494) - waiting to lock <0x00000007bc75a9a0> (a java.util.HashMap) at com.sun.jersey.server.impl.ejb.EJBInjectionInterceptor.get(EJBInjectionInterceptor.java:104) - locked <0x00000007bd113780> (a java.util.concurrent.ConcurrentHashMap) at com.sun.jersey.server.impl.ejb.EJBInjectionInterceptor.init(EJBInjectionInterceptor.java:71) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at com.sun.ejb.containers.interceptors.SystemInterceptorProxy.doCallback(SystemInterceptorProxy.java:133) at com.sun.ejb.containers.interceptors.SystemInterceptorProxy.init(SystemInterceptorProxy.java:120) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at com.sun.ejb.containers.interceptors.CallbackInterceptor.intercept(InterceptorManager.java:964) at com.sun.ejb.containers.interceptors.CallbackChainImpl.invokeNext(CallbackChainImpl.java:65) at com.sun.ejb.containers.interceptors.InterceptorManager.intercept(InterceptorManager.java:393) at com.sun.ejb.containers.interceptors.InterceptorManager.intercept(InterceptorManager.java:376) at com.sun.ejb.containers.StatelessSessionContainer.createStatelessEJB(StatelessSessionContainer.java:526) at com.sun.ejb.containers.StatelessSessionContainer.access$000(StatelessSessionContainer.java:95) at com.sun.ejb.containers.StatelessSessionContainer$SessionContextFactory.create(StatelessSessionContainer.java:724) at com.sun.ejb.containers.util.pool.NonBlockingPool.getObject(NonBlockingPool.java:247) at com.sun.ejb.containers.StatelessSessionContainer._getContext(StatelessSessionContainer.java:449) at com.sun.ejb.containers.BaseContainer.getContext(BaseContainer.java:2547) at com.sun.ejb.containers.BaseContainer.preInvoke(BaseContainer.java:1899) at com.sun.ejb.containers.EJBLocalObjectInvocationHandler.invoke(EJBLocalObjectInvocationHandler.java:212) at com.sun.ejb.containers.EJBLocalObjectInvocationHandlerDelegate.invoke(EJBLocalObjectInvocationHandlerDelegate.java:89) at com.sun.proxy.$Proxy523.handle(Unknown Source) [ -- removed stuff from our own namespace here -- ] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.glassfish.ejb.security.application.EJBSecurityManager.runMethod(EJBSecurityManager.java:1052) at org.glassfish.ejb.security.application.EJBSecurityManager.invoke(EJBSecurityManager.java:1124) at com.sun.ejb.containers.BaseContainer.invokeBeanMethod(BaseContainer.java:5388) at com.sun.ejb.EjbInvocation.invokeBeanMethod(EjbInvocation.java:619) at com.sun.ejb.containers.interceptors.AroundInvokeChainImpl.invokeNext(InterceptorManager.java:800) at com.sun.ejb.EjbInvocation.proceed(EjbInvocation.java:571) at org.jboss.weld.ejb.SessionBeanInterceptor.aroundInvoke(SessionBeanInterceptor.java:49) at sun.reflect.GeneratedMethodAccessor77.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at com.sun.ejb.containers.interceptors.AroundInvokeInterceptor.intercept(InterceptorManager.java:861) at com.sun.ejb.containers.interceptors.AroundInvokeChainImpl.invokeNext(InterceptorManager.java:800) at com.sun.ejb.EjbInvocation.proceed(EjbInvocation.java:571) at com.sun.ejb.containers.interceptors.SystemInterceptorProxy.doAround(SystemInterceptorProxy.java:162) at com.sun.ejb.containers.interceptors.SystemInterceptorProxy.aroundTimeout(SystemInterceptorProxy.java:149) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at com.sun.ejb.containers.interceptors.AroundInvokeInterceptor.intercept(InterceptorManager.java:861) at com.sun.ejb.containers.interceptors.AroundInvokeChainImpl.invokeNext(InterceptorManager.java:800) at com.sun.ejb.containers.interceptors.InterceptorManager.intercept(InterceptorManager.java:370) at com.sun.ejb.containers.BaseContainer.__intercept(BaseContainer.java:5360) at com.sun.ejb.containers.BaseContainer.intercept(BaseContainer.java:5348) at com.sun.ejb.containers.BaseContainer.callEJBTimeout(BaseContainer.java:4058) at com.sun.ejb.containers.EJBTimerService.deliverTimeout(EJBTimerService.java:1832) at com.sun.ejb.containers.EJBTimerService.access$100(EJBTimerService.java:108) at com.sun.ejb.containers.EJBTimerService$TaskExpiredWork.run(EJBTimerService.java:2646) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471) at java.util.concurrent.FutureTask.run(FutureTask.java:262) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:745) "main": at com.sun.jersey.server.impl.ejb.EJBInjectionInterceptor.get(EJBInjectionInterceptor.java:94) - waiting to lock <0x00000007bd113780> (a java.util.concurrent.ConcurrentHashMap) at com.sun.jersey.server.impl.ejb.EJBInjectionInterceptor.init(EJBInjectionInterceptor.java:71) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at com.sun.ejb.containers.interceptors.SystemInterceptorProxy.doCallback(SystemInterceptorProxy.java:133) at com.sun.ejb.containers.interceptors.SystemInterceptorProxy.init(SystemInterceptorProxy.java:120) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at com.sun.ejb.containers.interceptors.CallbackInterceptor.intercept(InterceptorManager.java:964) at com.sun.ejb.containers.interceptors.CallbackChainImpl.invokeNext(CallbackChainImpl.java:65) at com.sun.ejb.containers.interceptors.InterceptorManager.intercept(InterceptorManager.java:393) at com.sun.ejb.containers.interceptors.InterceptorManager.intercept(InterceptorManager.java:376) at com.sun.ejb.containers.StatelessSessionContainer.createStatelessEJB(StatelessSessionContainer.java:526) at com.sun.ejb.containers.StatelessSessionContainer.access$000(StatelessSessionContainer.java:95) at com.sun.ejb.containers.StatelessSessionContainer$SessionContextFactory.create(StatelessSessionContainer.java:724) at com.sun.ejb.containers.util.pool.NonBlockingPool.getObject(NonBlockingPool.java:247) at com.sun.ejb.containers.StatelessSessionContainer._getContext(StatelessSessionContainer.java:449) at com.sun.ejb.containers.BaseContainer.getContext(BaseContainer.java:2547) at com.sun.ejb.containers.BaseContainer.preInvoke(BaseContainer.java:1899) at com.sun.ejb.containers.EJBLocalObjectInvocationHandler.invoke(EJBLocalObjectInvocationHandler.java:212) at com.sun.ejb.containers.EJBLocalObjectInvocationHandlerDelegate.invoke(EJBLocalObjectInvocationHandlerDelegate.java:89) at com.sun.proxy.$Proxy332.create(Unknown Source) at com.sun.jersey.server.impl.container.filter.FilterFactory.getResourceFilters(FilterFactory.java:108) at com.sun.jersey.server.impl.model.method.ResourceHttpMethod.<init>(ResourceHttpMethod.java:72) at com.sun.jersey.server.impl.model.ResourceUriRules.processSubResourceMethods(ResourceUriRules.java:274) at com.sun.jersey.server.impl.model.ResourceUriRules.<init>(ResourceUriRules.java:139) at com.sun.jersey.server.impl.application.WebApplicationImpl.newResourceUriRules(WebApplicationImpl.java:709) at com.sun.jersey.server.impl.application.WebApplicationImpl.access$600(WebApplicationImpl.java:169) at com.sun.jersey.server.impl.application.WebApplicationImpl$9.f(WebApplicationImpl.java:553) at com.sun.jersey.server.impl.application.WebApplicationImpl$9.f(WebApplicationImpl.java:550) at com.sun.jersey.spi.inject.Errors.processWithErrors(Errors.java:193) at com.sun.jersey.server.impl.application.WebApplicationImpl.getUriRules(WebApplicationImpl.java:550) - locked <0x00000007bc75a9a0> (a java.util.HashMap) at com.sun.jersey.server.impl.application.WebApplicationImpl.initiateResource(WebApplicationImpl.java:657) at com.sun.jersey.server.impl.application.WebApplicationImpl.initiateResource(WebApplicationImpl.java:653) at com.sun.jersey.server.impl.application.RootResourceUriRules.<init>(RootResourceUriRules.java:124) at com.sun.jersey.server.impl.application.WebApplicationImpl._initiate(WebApplicationImpl.java:1298) at com.sun.jersey.server.impl.application.WebApplicationImpl.access$700(WebApplicationImpl.java:169) at com.sun.jersey.server.impl.application.WebApplicationImpl$13.f(WebApplicationImpl.java:775) at com.sun.jersey.server.impl.application.WebApplicationImpl$13.f(WebApplicationImpl.java:771) at com.sun.jersey.spi.inject.Errors.processWithErrors(Errors.java:193) at com.sun.jersey.server.impl.application.WebApplicationImpl.initiate(WebApplicationImpl.java:771) at com.sun.jersey.server.impl.application.WebApplicationImpl.initiate(WebApplicationImpl.java:766) at com.sun.jersey.spi.container.servlet.ServletContainer.initiate(ServletContainer.java:488) at com.sun.jersey.spi.container.servlet.ServletContainer$InternalWebComponent.initiate(ServletContainer.java:318) at com.sun.jersey.spi.container.servlet.WebComponent.load(WebComponent.java:609) at com.sun.jersey.spi.container.servlet.WebComponent.init(WebComponent.java:210) at com.sun.jersey.spi.container.servlet.ServletContainer.init(ServletContainer.java:373) at com.sun.jersey.spi.container.servlet.ServletContainer.init(ServletContainer.java:556) at javax.servlet.GenericServlet.init(GenericServlet.java:244) at org.apache.catalina.core.StandardWrapper.initServlet(StandardWrapper.java:1453) at org.apache.catalina.core.StandardWrapper.load(StandardWrapper.java:1250) - locked <0x00000007b7c8d9f8> (a org.apache.catalina.core.StandardWrapper) at org.apache.catalina.core.StandardContext.loadOnStartup(StandardContext.java:5093) at org.apache.catalina.core.StandardContext.start(StandardContext.java:5380) - locked <0x00000007b6233218> (a com.sun.enterprise.web.WebModule) at com.sun.enterprise.web.WebModule.start(WebModule.java:498) - locked <0x00000007b6233218> (a com.sun.enterprise.web.WebModule) at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:917) - locked <0x0000000701105360> (a java.util.LinkedHashMap) at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:901) at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:733) at com.sun.enterprise.web.WebContainer.loadWebModule(WebContainer.java:2019) at com.sun.enterprise.web.WebContainer.loadWebModule(WebContainer.java:1669) at com.sun.enterprise.web.WebApplication.start(WebApplication.java:109) at org.glassfish.internal.data.EngineRef.start(EngineRef.java:130) at org.glassfish.internal.data.ModuleInfo.start(ModuleInfo.java:269) - locked <0x0000000703307130> (a org.glassfish.internal.data.ModuleInfo) at org.glassfish.internal.data.ApplicationInfo.start(ApplicationInfo.java:301) at com.sun.enterprise.v3.server.ApplicationLifecycle.deploy(ApplicationLifecycle.java:461) at com.sun.enterprise.v3.server.ApplicationLoaderService.processApplication(ApplicationLoaderService.java:375) at com.sun.enterprise.v3.server.ApplicationLoaderService.postConstruct(ApplicationLoaderService.java:219) at com.sun.hk2.component.AbstractCreatorImpl.inject(AbstractCreatorImpl.java:131) at com.sun.hk2.component.ConstructorCreator.initialize(ConstructorCreator.java:91) at com.sun.hk2.component.AbstractCreatorImpl.get(AbstractCreatorImpl.java:82) at com.sun.hk2.component.SingletonInhabitant.get(SingletonInhabitant.java:67) - locked <0x00000007056a3c48> (a com.sun.hk2.component.SingletonInhabitant) at com.sun.hk2.component.EventPublishingInhabitant.get(EventPublishingInhabitant.java:139) at com.sun.hk2.component.AbstractInhabitantImpl.get(AbstractInhabitantImpl.java:78) at com.sun.enterprise.v3.server.AppServerStartup.run(AppServerStartup.java:253) at com.sun.enterprise.v3.server.AppServerStartup.doStart(AppServerStartup.java:145) at com.sun.enterprise.v3.server.AppServerStartup.start(AppServerStartup.java:136) - locked <0x00000007001a5628> (a com.sun.enterprise.v3.server.AppServerStartup) at com.sun.enterprise.glassfish.bootstrap.GlassFishImpl.start(GlassFishImpl.java:79) - locked <0x00000007001a5610> (a com.sun.enterprise.glassfish.bootstrap.GlassFishImpl) at com.sun.enterprise.glassfish.bootstrap.GlassFishDecorator.start(GlassFishDecorator.java:63) at com.sun.enterprise.glassfish.bootstrap.osgi.OSGiGlassFishImpl.start(OSGiGlassFishImpl.java:69) at com.sun.enterprise.glassfish.bootstrap.GlassFishMain$Launcher.launch(GlassFishMain.java:117) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at com.sun.enterprise.glassfish.bootstrap.GlassFishMain.main(GlassFishMain.java:97) at com.sun.enterprise.glassfish.bootstrap.ASMain.main(ASMain.java:55) Found 1 deadlock. ```
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The class
ComponentProcessorFactoryImplholds an internalConcurrentMap, that is being used to store component processors per source class. Thegetmethod uses a double-checked locking pattern to keep this map updated in a thread-safe manner, using locking withsynchronizedif the map does not already contain a map. If the map is not pre-filled, thegetmethod will populate it into the map. However, thesynchronizedlocking is done on the wrong object.This commit updates the double-checked locking so that the locking is done on the map being checked and updated, instead of locking on another field in the enclosing class.
This prevents a situation where two threads could deadlock each other. The following is an excerpt of a jstack dump from one of our production Glassfish servers: