RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

[Yasumasa Suenaga]

On 2020/03/27 9:01, Roger Riggs wrote:
> Hi,
> 
> Please don't throw an exception.
> It would abort being able to provide any information.
> And who would expect or handle such an exception?
> 
> Degrading the info or omitting it provides better service overall.
> There is no point to throwing an exception.

hProcess in QueryFullProcessImageNameW needs PROCESS_QUERY_INFORMATION or PROCESS_QUERY_LIMITED_INFORMATION access right, so I thought it is reasonable to throw runtime exception if the call failed.

Anyway, I agree with you if throwing exception is not comfortable in this case for Java API.


Thanks,

Yasumasa


> Regards, Roger
> 
> 
> On 3/26/20 7:55 PM, Yasumasa Suenaga wrote:
>> On 2020/03/27 0:35, Roger Riggs wrote:
>>> Hi,
>>>
>>> Reading further down the reference to the section:
>>> "Enable Long Paths in Windows 10, Version 1607, and Later"
>>> might suggest the code be more resilient to long paths.
>>>
>>> The stack allocated buffer (1024) is fine, but I'd suggest adding code to retry in the case
>>> of INSUFFICIENT BUFFER with a malloc'd buffer to make it more robust since the enabling
>>> of long paths is environment and application specific.
>>
>> In addition, it's better to throw an exception if the call failed due to other error code.
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>> Thanks, Roger
>>>
>>>
>>>
>>> On 3/26/20 10:40 AM, Toshio 5 Nakamura wrote:
>>>> Hi Thomas, Suenaga-san, Roger,
>>>>
>>>> Thank you for reviewing and comments.
>>>>
>>>> I checked behaviors by a small program and debugger.
>>>> If QueryFullProcessImageNameW failed by not enough buffer,
>>>> the API didn't change the buffer.
>>>> It just set ERROR_INSUFFICIENT_BUFFER(0x7a) to LastError.
>>>>
>>>> The buffer size becomes 1024 characters (2048 bytes) by this patch.
>>>> Should it use bigger size with malloc? 32,767 characters can be limit [2].
>>>> I feel 1024 is reasonable value for command's location.
>>>>
>>>> [2]
>>>> https://urldefense.com/v3/__https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file*maximum-path-length-limitation__;Iw!!GqivPVa7Brio!PgzCcUQOjCNpcVeetU_drS4DFFVLaj0ceJBvipX7iDc_RlPtcEkdH8AzelGtzqXS$
>>>> Best regards,
>>>> Toshio Nakamura
>>>>
>>>>> Hi,
>>>>>
>>>>> If the call fails, the command field in the info will not be set (and
>>>>> therefore null).
>>>>>
>>>>> I agree that a length of 512 (characters) is probably too small.
>>>>>
>>>>> Roger
>>>>>
>>>>>
>>>>> On 3/26/20 6:37 AM, Yasumasa Suenaga wrote:
>>>>>> Hi Nakamura-san,
>>>>>>
>>>>>> Your change almost looks good, but I have one question.
>>>>>> Length of exeName (1024) is enough?
>>>>>>
>>>>>> According to Microsoft Doc [1], exeName might not be null-terminated
>>>>>> if it failed.
>>>>>> I concern buffer overrun might occur when QueryFullProcessImageNameW
>>>>>> failed.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> [1]
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__docs.microsoft.com_en-2Dus_windows_win32_api_winbase_nf-2Dwinbase-2Dqueryfullprocessimagenamew&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=noFyLWDG1dMjO5TlaqM_zvW_f-8fvveS2EoZylT5DMQ&s=oc_KTGmJUjsfnLf3tjWr9DO1dwWp1TqIZgd2EiHVW14&e=
>>>>
>>>>>>
>>>>>> On 2020/03/26 17:58, Toshio 5 Nakamura wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Could you review this change? Additionally, I'd like to ask a sponsor
>>>>>>> for
>>>>>>> it, since I'm not a committer.
>>>>>>>
>>>>>>> Bug:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8232846&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=noFyLWDG1dMjO5TlaqM_zvW_f-8fvveS2EoZylT5DMQ&s=pn_uS0DojBk6-R6R5QwtYFJvJpaabhia-eiOtIrJO9Q&e=
>>>>
>>>>>>> Webrev:
>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Etnakamura_8232846_webrev.00&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=noFyLWDG1dMjO5TlaqM_zvW_f-8fvveS2EoZylT5DMQ&s=NylYK0Q3nFFZRVIaeUna6iaClaJm1CWJ2Zkwy-ysvv4&e=
>>>>
>>>>>>> Under Windows environments, ProcessHandle.Info.command shows question
>>>>>>> marks
>>>>>>> if the command has non-English characters. I'd like to change the
>>>>>>> underlying API to Unicode version.
>>>>>>> Tier1 tests passed.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Toshio Nakamura
>>>>>>>
>>>
>