RFR: JDK-8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file

[Alex Menkov]

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.spitsyn at 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
>