Responding to Chris's question below ...
On 30/04/2019 3:58 am, Chris Plummer wrote:
Hi Ralf,
I think print_debug_listen_address() should have some exception checking
added after the java calls.
I'm a little unsure why you modified DebugOnCmdStartDCmd to use
print_debug_listen_address(), but still have a fallback to print the
specified transport and address. If anything I would have written a
get_debug_listen_address() and used it to verify that the specified and
actual addresses end up being the same (and then also make
print_debug_listen_address() use this API).
I'm also unsure of your ThreadToNativeFromVM change. This is not an area
I understand well, so best to get someone else to ok it.
I can see that the new code for print_debug_listen_address needs to be
executed whilst still _thread_in_vm so the ThreadToNativeFromVM helper
can't extend over that part. However, I would suggest that you otherwise
keep it covering as much of the existing code as possible - specifically
the chunk that calls os::find_agent_function. I would be concerned that
the eventual calls to dlsym etc may potentially take some time and this
would prevent safepoints from occurring as this thread is still "in VM".
This might be overly conservative but it avoids any possibility of
introducing a change in behaviour for the existing code.
Cheers,
David
-----
You need to update copyright date to 2019.
Can you write a test for this new dcmd. You can probably just extend
OnJcmdTest.java.
thanks,
Chris
On 4/29/19 8:31 AM, Schmelter, Ralf wrote:
Thanks for the review.
I've update the webrev to use explicit NULL checks:
https://bugs.openjdk.java.net/browse/JDK-8223065
And I now use the pointer to the first byte in the result to split the
property value, since I might need the calculate the pointer past the
last character (if the prop ends with ':').
I cannot see the SEGV, but I've scheduled the patch to be tested in
our nightly build again, so maybe I can reproduce it there.
Best regards,
Ralf
