Make object cleaner thread name public#8014
Conversation
Motivation: The object cleaner thread can not be shutdown. This means that test suites that check for leaked threads fail on the object cleaner thread because it can not be shutdown at the end of a test. For such test suites, the object cleaner thread must be whitelisted to not fail thread leak control. This requires knowing the name of this thread which means that the name of this thread should be part of the public API. Modifications: Set the access modifier to public for the object cleaner thread name and add a test that asserts this thread name does not change. Result: The object cleaner thread name is part of the public API and subject to the same backwards compatibility guarantees as other components of the public API.
|
@jasontedor technically it is still subject to change as the class is in an "internal" package. |
|
Then I think this needs to be made fully part of the public API given the impact here if that's the only way to get BWC guarantees. Because of this thread, this was by far the most painful Netty upgrade that we have gone through. |
|
@jasontedor the reason why its in the internal package is basically because we dont want to have users register their own stuff as its easy to "mess-up" things. So I am not super happy with move it out of internal honestly. Maybe we can just allow you to adjust the name of the thread ? |
|
Well I am not happy with this thread leaking with no way to cleanly shut it down at the end of a test or otherwise control. We have thread leak control in place to reduce bugs and now we have this is the only thread we have to fight so hard with. This thread has leaked all over our test infrastructure now (elastic/elasticsearch#31232) so either there needs to be usability improvements so that we can revert this mess, or at a minimum some aspect of it become part of the public API so that have some confidence about minimizing backwards breaks. I would prefer that we be able to shut it down as we shutdown the thread death watcher and the global event executor in some of our test infrastructure today, but if that's not an option for some reason please make this part of the public API since it's an implementation detail that leaks. |
|
I am currently traveling so I will need a bit of time thinking about this. I wonder how you handle the new Cleaner stuff in Java 9 in terms of their thread . Can you explain how you handle it with this tests? |
|
That is not a thread created by test suites, that's a JVM thread created before test suites fires up and thread leak control starts tracking. The purpose of thread leak control (and other checks like this that we have) is that all resources (file handles, sockets, threads) created during the test are cleaned up at the end of the test (otherwise it could be indicative of a leak in production code). |
|
Got it thanks
… Am 12.06.2018 um 12:08 schrieb Jason Tedor ***@***.***>:
That is not a thread created by the test suite, that's a JVM thread created before the test suite fires up and thread leak control starts tracking. The purpose of thread leak control (and other checks like this that we have) is that all resources (file handles, sockets, threads) created during the test are cleaned up at the end of the test (otherwise it could be indicative of a leak in production code).
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Closing in favor of #8017 |
Motivation:
The object cleaner thread can not be shutdown. This means that test suites that check for leaked threads fail on the object cleaner thread because it can not be shutdown at the end of a test. For such test suites, the object cleaner thread must be whitelisted to not fail thread leak control. This requires knowing the name of this thread which means that the name of this thread should be part of the public API.
Modifications:
Set the access modifier to public for the object cleaner thread name and add a test that asserts this thread name does not change.
Result:
The object cleaner thread name is part of the public API and subject to the same backwards compatibility guarantees as other components of the public API.
Relates elastic/elasticsearch#31232