RFR: JDK-8188856: Incorrect file path in an exception message when .java_pid is not accessible on Unix
Gary Adams
gary.adams at oracle.com
Thu Dec 21 20:17:56 UTC 2017
A refreshed webrev is available
Webrev: http://cr.openjdk.java.net/~gadams/8188856/webrev.02/
- added solaris version of the file which had the same original error
reporting
the wrong socket file name
- restored the linux detach/execute logic
- updated the macosx and aix detach/execute logic
- a few minor differences between the various platform specific files
curly braces and exception args.
Ran into some issues with mach5 testing which will require another
test run tomorrow.
On 12/19/17, 10:00 AM, Gary Adams wrote:
> A refreshed webrev is available
> Webrev: http://cr.openjdk.java.net/~gadams/8188856/webrev.01/
>
> On 12/18/17, 4:38 PM, Chris Plummer wrote:
>> On 12/18/17 1:22 PM, gary.adams at oracle.com wrote:
>>> On 12/18/17 2:26 PM, Chris Plummer wrote:
>>>> Hi Gary,
>>>>
>>>> On 12/18/17 6:47 AM, Gary Adams wrote:
>>>>> Here's a simple fix to correct the error message when the java_pid
>>>>> socket
>>>>> is not found. The code previously reported the attach_pid file name
>>>>> rather than the socket file name that was not found.
>>>>>
>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8188856
>>>>> Webrev: http://cr.openjdk.java.net/~gadams/8188856/webrev.00/
>>>>
>>>> I don't understand why you couldn't simply have changed the
>>>> f.getPath() reference to "path". From what I can see, there is no
>>>> difference between "path" and "socket_name". The problem you are
>>>> fixing is that the error message prints f.getPath(), but that
>>>> refers to the attach file and the error message should refer to the
>>>> socket file. You've correct this, but have done so in a round about
>>>> way. Above the error message (in two places) exists:
>>>>
>>>> path = findSocketFile(pid, ns_pid);
>>>>
>>>> So "path" is what you want. You have indirectly fixed the problem
>>>> by having findSocketFile() store the path in socket_name, and then
>>>> you print socket_name, but why not just do the direct fix and print
>>>> "path".
>>>>
>>>> Also, the copyrights need to be updated.
>>>>
>>>> thanks,
>>>>
>>>> Chris
>>>>
>>>>
>>> The problem was path is used to hold the constructed file name
>>> if it is confirmed to exist in the file system. Otherwise it is
>>> passed back from
>>> findSocketFile as a null to indicate the socket file was not found.
>>>
>>> I could refactor where the existence check is handled, but it seemed
>>> like the least
>>> invasive change to simply save the socket name for the printing in
>>> the error case.
>>>
>> Ah, right. Obviously "path" is null when you want to print the error
>> message. I guess I have a hard time with "path" and "socket_name"
>> for the most part being the same (except "path" can end up being
>> null), yet they have two completely dissimilar names. Why not rename
>> path to socket_path and then add saved_socket_path, or something like
>> that. No changes to your current logic, just to the names being used.
>>
>> Or, have findSocketFile() actually return the File, and then rename
>> path to socket_file and also rename socket_name to socket_path. The
>> truth of the matter is the result of findSocketFile() is only used to
>> see if the socket file exists. You could even change it to return a
>> boolean.
>>
>> Chris
>>
>>
>
More information about the serviceability-dev
mailing list