<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">Hi Alex,<br>
<br>
I'm not sure this update suggested by Yasumasa is right:<br>
<pre><span class="new"> 536 // avoid deadlock if AttachListener thread is blocked at safepoint</span>
<span class="new"> 537 ThreadBlockInVM tbivm(JavaThread::current()); </span>
538 while (AttachListener::transit_state(AL_INITIALIZING,
539 AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) {
540 os::naked_yield();
541 }</pre>
<br>
The synchronization window becomes smaller with adding <span
class="new">ThreadBlockInVM,<br>
so we probably do not see the deadlock anymore.<br>
However, I think, we still need it inside the loop to work at
each iteration.<br>
Also, we do not care about performance here as it is extremely
rare case.</span> <br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 5/11/20 12:53, Alex Menkov wrote:<br>
</div>
<blockquote type="cite"
cite="mid:73e7edeb-664b-f381-1c4f-19181deecfbe@oracle.com">Hi
Yasumasa, Serguei, Chris,
<br>
<br>
Thank you for the feedback
<br>
updated webrev (all suggestions are applied):
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev.2/">http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev.2/</a>
<br>
<br>
On 05/11/2020 00:31, <a class="moz-txt-link-abbreviated" href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote:
<br>
<blockquote type="cite">Hi Alex,
<br>
<br>
It looks good in general.
<br>
I have a couple of minor comments.
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html">http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html</a>
+ // if for a reason the app hangs, we don't want to wait test
timeout
<br>
<br>
Nit: replace: wait test timeout => wait for test timeout
<br>
<br>
I hope, you remember about copyright comments update.
<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/src/hotspot/os/aix/attachListener_aix.cpp.udiff.html">http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/src/hotspot/os/aix/attachListener_aix.cpp.udiff.html</a>
<br>
<br>
Q1: How useful is this variable? :
<br>
AixAttachListener::_shutdown = false;
<br>
<br>
    Why is it needed on aix but not on other platforms?
<br>
</blockquote>
<br>
IFAIU AIX has issue with accept() - it hangs if the socket is
closed.
<br>
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.
<br>
<br>
--alex
<br>
<br>
<blockquote type="cite">
<br>
Thanks,
<br>
Serguei
<br>
<br>
<br>
On 5/8/20 18:14, Alex Menkov wrote:
<br>
<blockquote type="cite">Hi all,
<br>
<br>
please review the fix for
<br>
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8235211">https://bugs.openjdk.java.net/browse/JDK-8235211</a>
<br>
webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/">http://cr.openjdk.java.net/~amenkov/jdk15/RemovingUnixDomainSocket/webrev/</a>
<br>
<br>
Test failures are caused by deadlock during attach listener
restarting:
<br>
check_socket_file function aborts socket listening and waits
while attach listener state becomes AL_NOT_INITIALIZED (it
happens when AttachListener::dequeue returns NULL).
<br>
AttachListener::dequeue method is blocked in ThreadBlockInVM
dtor.
<br>
To solve it ThreadBlockInVM was added inside waiting cycle in
check_socket_file.
<br>
<br>
Other changes:
<br>
- made _listener (and _shutdown for aix) volatile as they are
used by 2 threads (attach listener thread and signal handler
thread) without synchronization;
<br>
- added close() for listening socket on aix (before it had
only shutdown() for it);
<br>
- additional logging and some cleanup in the test;
<br>
- added handling of LingeredApp hang.
<br>
<br>
--alex
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</body>
</html>