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