On Wed, 12 Jul 2023 11:56:18 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> test/jdk/java/util/concurrent/StructuredTaskScope/StressShutdown.java line 
>> 83:
>> 
>>> 81:             // fork subtask to shutdown
>>> 82:             scope.fork(() -> {
>>> 83:                 scope.shutdown();
>> 
>> Hello Alan, the proposed source change in `ThreadFlock` and the reason for 
>> that change appear good to me.
>> However, as far as I understand, it appears to me that this test may not be 
>> reproducing the issue where shutdown gets called when there are threads that 
>> are about to start. From what I see in the API and implementation of 
>> `scope.fork()`, when the `fork()` returns, it's guaranteed that a `Thread` 
>> for the subtask has been started (keeping aside the failed to start cases). 
>> So in this test here, when we reach this point where we attempt a 
>> `shutdown()` all 15 "beforeShutdown" subtasks would already have threads 
>> that are started and alive. i.e. there won't be any "about to be started 
>> thread". As for the 15 "afterShutdown" forks() that follow, they would 
>> already notice that the scope is shutdown and won't start the new threads.
>> Did I misunderstand this test?
>
>> As for the 15 "afterShutdown" forks() that follow, they would already notice 
>> that the scope is shutdown and won't start the new threads.
>> Did I misunderstand this test?
> 
> The test arranges for a subtask to do shutdown, it's not done by the main 
> task. That setups up the conditions for the shutdown to race with the 
> subsequent forking done by the main task.

Ah! Indeed, I missed that crucial detail. So the shutdown() call itself can 
happen when one of the "afterShutdown" subtask is being forked and a new thread 
being created for it. Makes sense. Thank you for that explanation.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14833#discussion_r1261073745

Reply via email to