Codereview request for 7171184: Make the failure message in 7168110 more specific

[Alan Bateman]

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20120523/2ca8a4ed/attachment.html