Updated webrev:
http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev.3/

--alex


On 05/11/2020 14:12, Alex Menkov wrote:
Please hold on with the review.

webrev.2 causes LingeredApp crash.
Looks like need to decrease ThreadBlockInVM scope.
Will send new version after completing test run.

On 05/11/2020 13:52, Chris Plummer wrote:

  228             // If the app hangs, we don't want to wait for the to test timeout.

Sorry, there was a typo in my suggestion. Should be "test to", not "to test".

Oh, I didn't pay enough attention to comment updates :)

--alex


thanks,

Chris

On 5/11/20 12:53 PM, Alex Menkov wrote:
Hi Yasumasa, Serguei, Chris,

Thank you for the feedback
updated webrev (all suggestions are applied):
http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev.2/

On 05/11/2020 00:31, serguei.spit...@oracle.com wrote:
Hi Alex,

It looks good in general.
I have a couple of minor comments.

http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html + // if for a reason the app hangs, we don't want to wait test timeout

Nit: replace: wait test timeout => wait for test timeout

I hope, you remember about copyright comments update.


http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/src/hotspot/os/aix/attachListener_aix.cpp.udiff.html

Q1: How useful is this variable? :
AixAttachListener::_shutdown = false;

     Why is it needed on aix but not on other platforms?

IFAIU AIX has issue with accept() - it hangs if the socket is closed.
I don't think this _shutdown flag helps a lot, but I don't want to make significant changes in the AIX code as I cannot test it.

--alex


Thanks,
Serguei


On 5/8/20 18:14, Alex Menkov wrote:
Hi all,

please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8235211
webrev:
http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/

Test failures are caused by deadlock during attach listener restarting: check_socket_file function aborts socket listening and waits while attach listener state becomes AL_NOT_INITIALIZED (it happens when AttachListener::dequeue returns NULL).
AttachListener::dequeue method is blocked in ThreadBlockInVM dtor.
To solve it ThreadBlockInVM was added inside waiting cycle in check_socket_file.

Other changes:
- made _listener (and _shutdown for aix) volatile as they are used by 2 threads (attach listener thread and signal handler thread) without synchronization; - added close() for listening socket on aix (before it had only shutdown() for it);
- additional logging and some cleanup in the test;
- added handling of LingeredApp hang.

--alex



Reply via email to