vyommani commented on code in PR #872:
URL: https://github.com/apache/ranger/pull/872#discussion_r3137185320


##########
agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestRangerRESTClientDeadlock.java:
##########
@@ -160,6 +160,7 @@ public void testInterruptWorksWithMultipleURLs() throws 
Exception {
         HttpServer httpServer2 = HttpServer.create(new InetSocketAddress(0), 
0);
         httpServer2.createContext("/", exchange -> {
             LOG.info("Server2: Received request, returning 503...");
+            serverReceivedRequest.countDown(); // Count down the latch for 
server2 as well

Review Comment:
   Since we are migrating to JDK 17, we need to be careful with changes to test 
synchronization like this. Could you explain why this specific countDown() was 
added to TestRangerRESTClientDeadlock? If the migration to JDK 17 introduced a 
race condition or a hang, we should document that in the code so we don't 
accidentally re-introduce the deadlock later. 
   Was this test failing specifically when you switched the build to JDK 17?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to