Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect ContextClassLoader for CommonJ WorkManager worker threads when Quartz Scheduler is used [SPR-11125] #15751

spring-projects-issues opened this issue Nov 26, 2013 · 5 comments


Copy link

@spring-projects-issues spring-projects-issues commented Nov 26, 2013

Peter H opened SPR-11125 and commented

When Quartz Scheduler is configured to utilize the CommonJ WorkManager, worker (WorkManager) threads have a different ContextClassLoader (CCL) than their caller.
This happens at WebSphere AS at least, not sure about WebLogic.

The reason is that Spring creates the main Quartz thread during scheduler startup (startScheduler method in SchedulerFactoryBean) on its own and that means that this thread is unmanaged.
This unmanaged thread is then used to control the all the Quartz activity and also to send jobs to the WorkManager.
While CCL is propagated properly from the any managed thread to an unmanaged thread, it is not propagated from any unmanaged thread to a managed thread.

This means that while the unmanaged Quartz main thread will have correct CCL, managed WorkManager threads will not. They will get SystemClassLoader instead.

This should be corrected so that jobs can execute with correct CCL and thus especially (hardly modifiable) 3rd party code that may utilizes this class loader won't fail.

Most likely, the run method in the DelegatingWork class should be modified to explicitly propagate the CCL. Also a cleanup action that resets the CCL back at the end of the run would be essential.

Affects: 3.0.7, 3.2.5

Reference URL:

2 votes, 4 watchers

Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 26, 2013

Peter H commented

Note that the same issue will probably affect also ConcurrentTaskExecutor / ManagedConcurrentTaskExecutor.

Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 26, 2014

Juergen Hoeller commented

Revisiting this one, it's actually a good old Quartz problem since it's Quartz internally starting that scheduler thread, with Spring historically not being able to do much about it. Granted, we could explicitly pass the CCL to worker threads, but wouldn't it be preferable for the Quartz scheduler thread to be managed to begin with? I'll double-check Quartz 2.2 whether there's any way to override the scheduler thread now.

Note that these problems should not exist when using Spring's own scheduling support (@Scheduled etc). It's a Quartz-specific issue.


Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 10, 2014

Peter H commented

This unmanaged threads problem causes also another issues (for example with JNDI lookups) so I took another look at it and there's actually a possibility to run all the Quartz controller threads as managed.
What has to be done in addition to providing the taskExecutor instance (in this case the WorkManagerTaskExecutor implementation) to the SchedulerFactoryBean instance:

Give (to Quartz) the ThreadExecutor, in this case via Quartz properties org.quartz.threadExecutor.class=org.quartz.commonj.WorkManagerThreadExecutor and org.quartz.threadExecutor.workManagerName=

After this, it will run even its own controller threads on that work manager and things will be much better (CCL and Java EE context aren't lost) .

There are few culprits however

  • the current (as of Quartz up to 2.2.1) WorkManagerThreadExecutor implementation needs one little fix in the DelegatingWork.isDaemon() method to return true (true is for long running work, false for short) to avoid hung thread notifications such as here - so for now you should use your own implementation that extends this one and overrides at least that method. (See also your DelegatingWork implementation)
  • the configuration is quite cumbersome now, you need to provide taskExecutor to the SchedulerFactoryBean (that's fine) and then the appropriate threadExecutor to the Quartz itself. Can't this be done right away by the SchedulerFactoryBean itself, similarly to setting the LocalDataSourceJobStore as the Quartz job store implementation if the dataSource is provided?
  • there's currently no implementation of JSR-236 ManagedExecutorService for this, it would be nice to have one at least here (so that we won't have to depend on what Quartz provides as things seem to proceed very slowly over there). And then SchedulerFactoryBean would pick the right one depending on the provided taskExecutor.

Note that I've found also this article where the use of fully qualified JNDI name (indirect lookup) such as java:comp/env/wm/default is discouraged and the short one (direct lookup) like wm/default is recommended. Well, I did everything with the full name and didn't have any problem and don't see any good reason why there should be any (if you've got a resource reference for that work manager). You could probably make use of the resourceRef property (JndiLocatorSupport) here.

Note about the Quartz control threads (for those curious about how many of them may exist and how to determine the thread pool sizing):

  • Each Quartz Scheduler instance creates one main scheduler thread
  • If JDBC job store is configured, an additional MisfireHandler thread is created
  • if isClustered is set (in addition to the JDBC job store of course), an additional ClusterManager thread is created

So there could be 1-3 such threads depending on the scheduler configuration.

All the above is possible with Quartz 1.8.6 and 2.1.0 or later:

Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 12, 2019

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.

Copy link

@mag01 mag01 commented Jan 15, 2019

I'm original reporter of the issue and yes, it's definitely still relevant. I don't seem to be able to directly reopen this issue here though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants