[
https://issues.apache.org/jira/browse/KAFKA-20802?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Eric Chang updated KAFKA-20802:
-------------------------------
Description:
{{ClusterTestExtensions#afterEach}} builds the thread-leak error message with
{{t.getThreadGroup().getName()}}. {{Thread#getThreadGroup()}} returns null
after a thread has terminated.
If a thread returned by {{DetectThreadLeak#newThreads()}} terminates between
the snapshot and message formatting, {{afterEach}} throws an NPE instead of
producing the thread-leak report.
h3. CI evidence
* Full job:
https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389
* NPE in the JUnit test step:
https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389#step:6:136310
* Flaky-test report:
https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389#step:10:1872
* Affected test: {{MetadataQuorumCommandTest.testDescribeQuorumStatusSuccessful
[5]}}
{code}
java.lang.NullPointerException: Cannot invoke "java.lang.ThreadGroup.getName()"
because the return value of "java.lang.Thread.getThreadGroup()" is null
at
org.apache.kafka.common.test.junit.ClusterTestExtensions.lambda$afterEach$2(ClusterTestExtensions.java:188)
{code}
h3. Reproduction
A deterministic reproducer can make {{DetectThreadLeak#newThreads()}} return a
thread that has already been joined, then invoke the real {{afterEach}} path.
The old implementation throws the same NPE. The fix preserves the detected
thread in the leak report and formats a missing thread-group name as
{{<terminated>}}.
I also ran the actual affected test locally for 120 isolated cluster contexts
using Java 17, 4 forks, and retries disabled. All 120 passed, so the natural
timing window did not reproduce locally in that batch.
h3. Expected behavior
The leak-reporting path should not throw an NPE when
{{Thread#getThreadGroup()}} returns null. It should preserve the detected
thread and use {{<terminated>}} as the thread-group name.
h3. Related issues
KAFKA-20619 (https://issues.apache.org/jira/browse/KAFKA-20619) introduced
thread-group names in the leak report by calling
{{t.getThreadGroup().getName()}}, which made this race surface as an NPE.
KAFKA-17189 (https://issues.apache.org/jira/browse/KAFKA-17189) discusses the
related thread-liveness race and proposes adding {{Thread::isAlive}} to the
leak-detector filter, but it did not introduce this NPE.
was:
{{ClusterTestExtensions#afterEach}} builds the thread-leak error message with
{{t.getThreadGroup().getName()}}. {{Thread#getThreadGroup()}} returns null
after a thread has terminated.
If a thread returned by {{DetectThreadLeak#newThreads()}} terminates between
the snapshot and message formatting, {{afterEach}} throws an NPE and fails the
test even though the thread is no longer a leak.
h3. CI evidence
* Full job:
https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389
* NPE in the JUnit test step:
https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389#step:6:136310
* Flaky-test report:
https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389#step:10:1872
* Affected test: {{MetadataQuorumCommandTest.testDescribeQuorumStatusSuccessful
[5]}}
{code}
java.lang.NullPointerException: Cannot invoke "java.lang.ThreadGroup.getName()"
because the return value of "java.lang.Thread.getThreadGroup()" is null
at
org.apache.kafka.common.test.junit.ClusterTestExtensions.lambda$afterEach$2(ClusterTestExtensions.java:188)
{code}
h3. Reproduction
A deterministic regression test can reproduce the same NPE by making
{{DetectThreadLeak#newThreads()}} return a thread that has already been joined,
then invoking the real {{afterEach}} path. The proposed logic passes this
regression test and continues to report live leaked threads.
I also ran the actual affected test locally for 120 isolated cluster contexts
using Java 17, 4 forks, and retries disabled. All 120 passed, so the natural
timing window did not reproduce locally in that batch.
h3. Expected behavior
Threads that have already terminated should be ignored while constructing the
leak report because they are not leaks. The reporting path must not throw an
NPE.
h3. Related issue
KAFKA-17189 (https://issues.apache.org/jira/browse/KAFKA-17189) describes the
underlying snapshot-to-report race and proposes adding {{Thread::isAlive}} to
the leak-detector filter. This issue tracks the concrete
{{ClusterTestExtensions#afterEach}} NPE observed in CI.
> ClusterTestExtensions#afterEach throws NPE when a detected thread terminates
> before the leak report is built
> ------------------------------------------------------------------------------------------------------------
>
> Key: KAFKA-20802
> URL: https://issues.apache.org/jira/browse/KAFKA-20802
> Project: Kafka
> Issue Type: Bug
> Reporter: Eric Chang
> Assignee: Eric Chang
> Priority: Major
> Fix For: 4.4.0
>
>
> {{ClusterTestExtensions#afterEach}} builds the thread-leak error message with
> {{t.getThreadGroup().getName()}}. {{Thread#getThreadGroup()}} returns null
> after a thread has terminated.
> If a thread returned by {{DetectThreadLeak#newThreads()}} terminates between
> the snapshot and message formatting, {{afterEach}} throws an NPE instead of
> producing the thread-leak report.
> h3. CI evidence
> * Full job:
> https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389
> * NPE in the JUnit test step:
> https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389#step:6:136310
> * Flaky-test report:
> https://github.com/apache/kafka/actions/runs/29228499012/job/86748625389#step:10:1872
> * Affected test:
> {{MetadataQuorumCommandTest.testDescribeQuorumStatusSuccessful [5]}}
> {code}
> java.lang.NullPointerException: Cannot invoke
> "java.lang.ThreadGroup.getName()" because the return value of
> "java.lang.Thread.getThreadGroup()" is null
> at
> org.apache.kafka.common.test.junit.ClusterTestExtensions.lambda$afterEach$2(ClusterTestExtensions.java:188)
> {code}
> h3. Reproduction
> A deterministic reproducer can make {{DetectThreadLeak#newThreads()}} return
> a thread that has already been joined, then invoke the real {{afterEach}}
> path. The old implementation throws the same NPE. The fix preserves the
> detected thread in the leak report and formats a missing thread-group name as
> {{<terminated>}}.
> I also ran the actual affected test locally for 120 isolated cluster contexts
> using Java 17, 4 forks, and retries disabled. All 120 passed, so the natural
> timing window did not reproduce locally in that batch.
> h3. Expected behavior
> The leak-reporting path should not throw an NPE when
> {{Thread#getThreadGroup()}} returns null. It should preserve the detected
> thread and use {{<terminated>}} as the thread-group name.
> h3. Related issues
> KAFKA-20619 (https://issues.apache.org/jira/browse/KAFKA-20619) introduced
> thread-group names in the leak report by calling
> {{t.getThreadGroup().getName()}}, which made this race surface as an NPE.
> KAFKA-17189 (https://issues.apache.org/jira/browse/KAFKA-17189) discusses the
> related thread-liveness race and proposes adding {{Thread::isAlive}} to the
> leak-detector filter, but it did not introduce this NPE.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
