On Wed, 12 Jul 2023 11:56:18 GMT, Alan Bateman <[email protected]> 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