Skip to content

Conversation

@akarnokd
Copy link
Member

Proposal for shutting down the computation and io schedulers on demand.
#1730

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like putting a synchronized block in the line of fire on something like this that doesn't need to be.

This is a global lock and exactly the type of stuff I find in our production environments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course we can do shutdown without this, but then the schedulers are dead and without app restrart, they won't come back.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to mention, they can't be tested because they shutdown the global threads for good.

Schedulers.io() cost only a volatile read and doesn't block concurrent
calls unnecessary.
@benjchristensen benjchristensen modified the milestone: 1.1 Nov 9, 2014
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have some javadoc on this method about if this method is synchronous or asynchronous? If it's not synchronous then how will I know when the Scheduler has shutdown, that is when the running threads currently associated with the Scheduler have completed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not detailed in the requirements, but the implementation basically calls unsubscribe() and shutdownNow() which generally do best effort to terminate outstanding work. So it is quite possible an unresponsive/ignorant work may keep a worker thread operational and make Tomcat still print out the leak notification.

Is the underlying issue still relevant (almost a month passed with no feedback)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants