Hi Yasumasa,
On 5/11/20 17:08, Yasumasa Suenaga wrote:
On 2020/05/12 9:01, Alex Menkov wrote:
Hi Serguei,
If I understand correctly, this also should work - when we mark a
thread "_thread_blocked" VM does not need to suspend it at safepoint.
I think so, but if state would be transit to the other in
is_init_trigger(), it might be affect to incorrect behavior.
So I suggested to wrap ThreadInVM and while loop with code block ({}).
Then I do not see it a problem to put helper inside the loop.
In normal case, just one iteration is expected.
I do not see any potential performance problem here.
Thanks,
Serguei
Thanks,
Yasumasa
--alex
On 05/11/2020 15:36, serguei.spit...@oracle.com wrote:
Hi Alex,
I'm not sure this update suggested by Yasumasa is right:
536 // avoid deadlock if AttachListener thread is blocked at safepoint
537 ThreadBlockInVM tbivm(JavaThread::current());
538 while (AttachListener::transit_state(AL_INITIALIZING,
539 AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) {
540 os::naked_yield();
541 }
The synchronization window becomes smaller with adding ThreadBlockInVM,
so we probably do not see the deadlock anymore.
However, I think, we still need it inside the loop to work at each
iteration.
Also, we do not care about performance here as it is extremely rare
case.
Thanks,
Serguei
On 5/11/20 12:53, 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
