RFR: JDK-8188856: Incorrect file path in an exception message when .java_pid is not accessible on Unix

[Gary Adams]

A refreshed webrev is available
     Webrev: http://cr.openjdk.java.net/~gadams/8188856/webrev.03/

  - "String socket_path" is now used where the original "path" variable 
was used
  - "File socket_file" is used for the exists() and getPath() calls
  - left solaris openDoor optimizations out, because it is not really 
related
     to the original reported issue
  - left linux findSocketFile as a separate method, because it contains
     extra nspid cgroup aware logic. The original findSocketFile performed
     2 steps to formulate the java_pid path name and to check existence.
     Now that it only formulates the path name, it seems reasonable to 
fold the
     processing into the caller. There already was an asymmetry in the 
platform
     specific implementations as solaris was using openDoor and the open 
file
     descriptor for it's connected flag checks.

Bypassed the mach5 errors I was seeing by moving over to testing with
jdk/jdk repos.

If this is acceptable, I'll create an hg export patch file for
my sponsor.

On 12/21/17, 3:17 PM, Gary Adams wrote:
> 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
>>>
>>>
>>
>