On Fri, 20 Mar 2026 12:37:09 GMT, Daniel Fuchs <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - await for the server thread to complete
>> - Daniel's review - fail the test if there's an exception on the server side
>
> test/jdk/java/net/DatagramPacket/ReuseBuf.java line 72:
>
>> 70: System.err.println("server exception: " + e);
>> 71: e.printStackTrace();
>> 72: throw new RuntimeException(e);
>
> Throwing an exception in a Thread might not result in the test failing. Have
> you considered using a CompletableFuture or AtomicReference instead to get
> potential exceptions from the main thread?
> I see this part is pre-existing, so maybe it's not an issue to continue
> throwing here.
Done, updated the test to check for the server side failure (if any) and fail
the test. The test continues to pass. I'll run a test-repeat with this change
in our CI.
> test/jdk/java/net/DatagramPacket/ReuseBuf.java line 118:
>
>> 116: InetSocketAddress destAddr = server.getServerAddress();
>> 117: // start the server
>> 118: new Thread(server).start();
>
> Ideally something should join that thread at the end. Not joining threads
> created by tests at the end of the tests has been known to cause issues,
> especially when tests are not run in /othervm.
That's a good point. Fixed in the updated PR.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30335#discussion_r2965634738
PR Review Comment: https://git.openjdk.org/jdk/pull/30335#discussion_r2965626519
