[jira] [Updated] (HBASE-30317) TestExecutorService.testExecutorService has a synchronization race when checking the executor status dump

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HBASE-30317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

ASF GitHub Bot updated HBASE-30317:
-----------------------------------
    Labels: pull-request-available  (was: )

> TestExecutorService.testExecutorService has a synchronization race when checking the executor status dump
> ---------------------------------------------------------------------------------------------------------
>
>                 Key: HBASE-30317
>                 URL: https://issues.apache.org/jira/browse/HBASE-30317
>             Project: HBase
>          Issue Type: Bug
>          Components: test
>    Affects Versions: 3.0.0-alpha-1, 4.0.0-alpha-1, 2.5.10, 2.6.5
>            Reporter: mazhengxuan
>            Assignee: mazhengxuan
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 4.0.0-alpha-1
>
>
> h3. Problem
> TestExecutorService.testExecutorService has a synchronization race when it checks the executor status dump.
> The test submits five TestEventHandler instances and waits only until counter reaches five. Each handler increments the counter immediately after entering process():
> {code:java}
> int num = counter.incrementAndGet();
> LOG.info("Running process #" + num + ", threadName=" + Thread.currentThread().getName());
> synchronized (lock) {
>   while (lock.get()) {
>     lock.wait();
>   }
> }
> {code}
> The main test thread treats counter == 5 as proof that all handlers are waiting on lock, captures ExecutorStatus, and expects the dump to contain:
> {code}
> Waiting on java.util.concurrent.atomic.AtomicBoolean
> {code}
> However, counter == 5 proves only that all handlers have started. It does not prove that they have completed the logging expression and entered lock.wait().
> Under resource pressure, the main test thread can capture the status while all five handlers are still RUNNABLE. In the reproduced failure, all handler stacks were executing:
> {code}
> StringConcatFactory.makeConcatWithConstants(...)
> {code}
> from the first LOG.info expression. None of them had reached lock.wait(), so checkStatusDump failed to find the expected text.
> h3. Reproduction
> The failure was reproduced independently with:
> {code:bash}
> mvn \
>   -pl hbase-server -am \
>   -Dtest=org.apache.hadoop.hbase.executor.TestExecutorService \
>   -Dsurefire.failIfNoSpecifiedTests=false \
>   test
> {code}
> Result:
> {code}
> Tests run: 3, Failures: 1, Errors: 0
> {code}
> Observed system load:
> {code}
> SystemLoadAverage=471
> AvailableMemoryMB=21
> ProcessCount=574
> {code}
> The high load makes the race much easier to reproduce, but the test itself contains the synchronization defect.
> h3. Expected behavior
> The status dump should be checked only after the handlers have actually entered the WAITING state on the AtomicBoolean monitor.
> h3. Proposed fix
> Before the final assertion, poll ExecutorStatus until its dump contains the expected waiting state, subject to a bounded timeout. Then run the existing final assertions.
> A fixed Thread.sleep should not be added because it would only reduce the probability of the race and would remain unreliable on slow or overloaded machines.
> h3. Related issues
> * HBASE-30303: this race was discovered while validating its changes.
> * HBASE-23241: fixed a different synchronization race in TestExecutorService.testSnapshotHandlers; this issue is not a duplicate.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)