Hi Serguei,
Can I send RFR for JDK-8225690 and close JDK-8225193 as duplicate of it?
Thanks,
Yasumasa
On 2019/07/03 1:06, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
The approach looks Okay in general.
Could you, please, post an RFR with the bug number in the email subject?
Thanks,
Serguei
On 6/26/19 18:41, Yasumasa Suenaga wrote:
Hi Serguei,
Thank you for your comment.
I uploaded new webrev which includes the fix.
http://cr.openjdk.java.net/~ysuenaga/JDK-8225690/proposal.3/
Also I added TODO for other Un*x OSes.
If the change for Linux is OK, I will copy it to others.
(I do not have them, so I depend on submit repo.)
This webrev has passed test/hotspot/jtreg/serviceability/attach and
test/jdk/com/sun/tools/attach jtreg tests on Linux x64.
Thanks,
Yasumasa
2019年6月27日(木) 6:10 serguei.spit...@oracle.com <serguei.spit...@oracle.com>:
Hi Yasumasa,
I'm reviewing it.
Just a quick comment.
http://cr.openjdk.java.net/~ysuenaga/JDK-8225690/proposal.2/src/hotspot/os/linux/attachListener_linux.cpp.udiff.html
+ // reads a request from the given connected socket
+ static LinuxAttachOperation* read_request(int s);
+
+ static bool _atexit_registered;
+
+ public:
+ enum {
+ ATTACH_PROTOCOL_VER = 1 // protocol version
+ };
+ enum {
+ ATTACH_ERROR_BADVERSION = 101 // error codes
+ };
+
static void set_path(char* path) {
if (path == NULL) {
+ _path[0] = '\0';
_has_path = false;
} else {
strncpy(_path, path, UNIX_PATH_MAX);
_path[UNIX_PATH_MAX-1] = '\0';
_has_path = true;
}
}
static void set_listener(int s) { _listener = s; }
- // reads a request from the given connected socket
- static LinuxAttachOperation* read_request(int s);
-
- public:
- enum {
- ATTACH_PROTOCOL_VER = 1 // protocol version
- };
- enum {
- ATTACH_ERROR_BADVERSION = 101 // error codes
- };
-
You moved public definitions of enums up in the code.
All the declarations below that were private before became public now.
Not sure, if it was your intention
Also, the Copyright comment in this file needs an update.
Thanks,
Serguei
On 6/26/19 07:34, Yasumasa Suenaga wrote:
Hi,
I'm not clear how this addresses the issue with deleting the file? The
file still has to exist IIUC for the mechanism to work.
This seems more suited to fixing the "multiple attach threads" problem.
How about this change?
http://cr.openjdk.java.net/~ysuenaga/JDK-8225690/proposal.2/
I think it can fix both JDK-8225690 and JDK-8225193.
This webrev implements for Linux only.
If we work further based on this, we need to implement
AttachListener::check_socket_file() for each OS's implementation.
Thanks,
Yasumasa
On 2019/06/21 23:16, Yasumasa Suenaga wrote:
On 2019/06/21 22:51, David Holmes wrote:
Hi Yasumasa,
On 21/06/2019 5:12 am, Yasumasa Suenaga wrote:
Hi,
Can we fix this issue like this webrev?
http://cr.openjdk.java.net/~ysuenaga/JDK-8225690/proposal/
I'm not clear how this addresses the issue with deleting the file? The file
still has to exist IIUC for the mechanism to work.
This seems more suited to fixing the "multiple attach threads" problem.
Yes, my proposal focuses to "multiple attach threads".
I shared my patch because it might help the work for this issue.
This patch would guard multithread-issue in Attach Listener.
So unix domain socket file will create just once.
Yasumasa
David
I could reproduce this issue with ConcAttachTest.java in it
on my laptop.
(Fedora 30 x64 on Hyper-V guest: Core i3-8145U x 2vcpu)
If we need to fix based on current implementation, I think
we need to use atomic operation.
But if we can refactor around here, we might be able to another approach - e.g.
using monitors/mutexes
Thanks,
Yasumasa
On 2019/06/21 5:49, David Holmes wrote:
Sorry it took me a while to understand the specifics of the problem. :)
David
On 20/06/2019 3:37 am, nijiaben wrote:
Yes Alan, I mean this
------------------?Original?------------------
*From: *?"Alan Bateman"<Alan.Bateman at oracle.com>;
*Date: *?Thu, Jun 20, 2019 02:54 PM
*To: *?"nijiaben"<nijiaben at perfma.com>; "David
Holmes"<david.holmes at oracle.com>;
"serviceability-dev"<serviceability-dev at openjdk.java.net>;
"jdk8u-dev"<jdk8u-dev at openjdk.java.net>;
"hotspot-runtime-dev"<hotspot-runtime-dev at openjdk.java.net>;
*Subject: *?Re: A Bug about the JVM Attach mechanism
On 20/06/2019 05:10, nijiaben wrote:
> :
> I know this mechanism, can we provide means of recovery to avoid
unavailability caused by accidental deletion?
>
Are you concerned about tmpreaper or cron jobs that periodically cleanup
/tmp? There may indeed be an issue for applications that run for weeks
or months. If someone is using jmap, jcmd or other tools using the
attach API then it will trigger the attach listener to start. When they
come back in a few weeks then the .java_pid<pid> file may have been
removed so they cannot attach. Is this what what you are pointing out?
-Alan
