On Mon, 24 Mar 2025 12:12:10 GMT, Michael McMahon <micha...@openjdk.org> wrote:

>> Guards against multi-deletes in `UnixSocketChannelReadWrite`.
>> 
>> The fix can be verified as follows:
>> 
>> 
>> make build-microbenchmark
>> build/linux-x64/jdk/bin/java \
>>   -jar build/linux-x64/images/test/micro/benchmarks.jar \
>>   -f 1 -wi 1 -i 1 -t 2 UnixSocketChannelReadWrite
>> 
>> 
>> Note that `-t 2` denotes the number of worker threads, i.e., parallelism.
>
>> > > Seems like the only explanation for why that is happening is someone 
>> > > cleaning out /tmp while the test is running
>> > 
>> > 
>> > @Michael-Mc-Mahon No, the reason for the failure is multiple threads 
>> > creating a Unix socket in the very same `tempDir`, and then trying to 
>> > delete the `tempDir`. See the following execution flow involving multiple 
>> > threads:
>> > ```
>> > 1. `tempDir` is statically assigned during class initialization (say 
>> > `/tmp/readWriteTest2414375588689416060`)
>> > 
>> > 2. `Thread1` runs `beforeRun()`, which creates the Unix socket 
>> > `/tmp/readWriteTest2414375588689416060/1`
>> > 
>> > 3. `Thread2` runs `beforeRun()`, which creates the Unix socket 
>> > `/tmp/readWriteTest2414375588689416060/2`
>> > 
>> > 4. Both `Thread1` and `Thread2` runs `afterRun()` in parallel
>> > 
>> > 5. `Thread1` runs `Files.delete(/tmp/readWriteTest2414375588689416060/1)` 
>> > and succeeds
>> > 
>> > 6. `Thread2` runs `Files.delete(/tmp/readWriteTest2414375588689416060/2)` 
>> > and succeeds
>> > 
>> > 7. `Thread1` runs `Files.delete(/tmp/readWriteTest2414375588689416060)` 
>> > and succeeds (since both `/1` and `/2` socket files are removed above, 
>> > and, hence, the directory is empty)
>> > 
>> > 8. `Thread2` runs `Files.delete(/tmp/readWriteTest2414375588689416060)` 
>> > and fails, since the directory has already been deleted above
>> > ```
>> > 
>> > 
>> >     
>> >       
>> >     
>> > 
>> >       
>> >     
>> > 
>> >     
>> >   
>> > Ideally, `tempDir` deletion should be performed by a single thread. But 
>> > replacing `Files::delete` with `Files::deleteIfExists` also does the job, 
>> > at a lower cost.
>> 
>> What would happen then if thread1 completes before thread2 starts? Maybe, we 
>> _should_ delete the directory in a separate block, executed in a single 
>> thread at the every end. Is there a variant of the @teardown() annotation 
>> which executes at the end of the test?
> 
> maybe the directory could be created and deleted in separate 
> Setup/Teardown(Level.Trial) invocations ?

@Michael-Mc-Mahon, I use dedicated temporary directory per socket in 
cabb4145759a982ba32e9ade82066cfc75b5f05d. This reduces the amount of 
boilerplate for socket file administration (-13 LoC), and avoids any 
synchronization due to its shared-nothing state. Is this okay with you?

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

PR Comment: https://git.openjdk.org/jdk/pull/24126#issuecomment-2749159972

Reply via email to