On Fri, 9 May 2025 13:01:20 GMT, Michael McMahon <micha...@openjdk.org> wrote:
>>> It's not clear to me why each `ServerSocketChannel` needs a separate >>> temporary directory for its socket file. >> >> Each JMH thread needs its own socket file. If we share the socket file >> folder, the following operations become error-prone due to racy nature of >> parallel JMH execution: >> >> 1. Creation of the parent folder >> 2. Creation of socket files (names shouldn't conflict) >> 3. Deletion of the parent folder (Parent can only be deleted when all >> workers are finished and their associated socket files are removed.) >> >> Following tickets were created due to such (some stable, some intermittent) >> errors: >> >> - `NoSuchFileException` error reported in the ticket this PR is addressing >> - [8350915: JMH: test SocketChannelConnectionSetup failed for 2 threads >> config](https://bugs.openjdk.org/browse/JDK-8350915) >> - [8351601: JMH: test UnixSocketChannelReadWrite failed for 2 threads >> config](https://bugs.openjdk.org/browse/JDK-8351601) >> >> To avoid all this, we create a dedicated socket folder for each socket of >> each JMH worker. The implemented nothing-shared approach renders >> synchronization redundant, and hence, avoids its racy pitfalls. >> >>> Also, is it possible to combine the functionality of the >>> `ServerSocketChannelHolder` defined in one of the tests, with this class? >> >> `ServerUdsChannelHolder` simplifies the Unix domain (UD) socket termination >> complexity caused by *files* involved. `SocketChannelConnectionSetup` >> benchmark employs both IP and UD sockets. There `ServerSocketChannelHolder` >> is an adapter for the termination logic, delegating to >> `ServerUdsChannelHolder` for UD sockets. It is possible to extend >> `ServerUdsChannelHolder` to also accept IP sockets, but I doubt if it worth >> the added complexity. Would you like me to give that a try? > > What about just doing `bind(null)`? This will create all the socket files > with unique names in the system temp directory. Since no sub-directory is > being created, there can't be a race trying to delete it. > > Or maybe this is what the test was doing originally and it ran into some > other problem? I thought `bind(null)` was not an option, since all UDS-involving benchmarks were designed with explicit file paths. Though upon closer look, I don't see any reason why those can't simply be replaced with `bind(null)` – implemented in 0b33d9321dd. Thanks to your suggestion, now this PR brings even a healthier diet: 2 files changed, 15 insertions(+), 54 deletions(-) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24568#discussion_r2084028318
