Hi Alan,
Sorry, you've picked me up on that "if (" space a number of times.
Thanks for pointing it out again.
What should I be checking w.r.t. the ProcessIdToSessionId? It returns a
0 on failure so once the session id's are unequal or we've encountered
an ERROR_ACCESS_DENIED we should really be good to go right?
Looking at that reminds me that I had got an && instead of an || for the
session id / error check. Its a good point that if the
ProcessIdToSessionId call fails for a reason outside ACCESS_DENIED, we
won't know about it. I'm wondering if I should revert that now?
-Rob
On 23/05/12 19:58, Alan Bateman wrote:
On 23/05/2012 18:44, Rob McKenna wrote:
Hi folks,
David Holmes suggested a rewrite of the original bug (7168110) so
I've put together the following:
http://cr.openjdk.java.net/~robm/7171184/webrev.01/
It will narrow down the cause of the error and return a more specific
message when there is a problem attaching jstack to a process created
in another session.
Let me know what you think,
I wasn't aware of ProcessIdToSessionId but seems a good idea.
I assume you should check the return value from ProcessIdToSessionId
before looking at the session id or last error.
Minor nit but you are missing a space in "if(" (line 479)
One idea to avoid duplicating the cleanup (VirtualFreeEx) code is:
if (GetLastError() == ERROR_NOT_ENOUGH_MEMORY) {
if (( ProcessIdToSessionId( GetCurrentProcessId(), &cSid) != 0) {
BOOL result = ProcessIdToSessionId(GetProcessId(hProcess), &pSid);
if ((result != 0 && (pSid != cSid)) || GetLastError() ==
ERROR_ACCESS_DENIED)) {
JNU_ThrowIOException(env, "useful message");
SetLastError(0);
}
}
}
if (GetLastError() != 0)
JNU_ThrowIOExceptionWithLastError(env, "CreateRemoteThread failed");
Just a suggestion so that you have one return path. The one downside
(and you've got this issue already) is that if ProcessIdToSessionId
fails with something other access denied then the error for the
exception wont' be right.
-Alan.
