RFR(S): JDK-8157236 - attach on ARMv7 fails with com.sun.tools.attach.AttachNotSupportedException: Unable to open socket file

[Dmitry Samersoff]

David,

> Thanks, this looks okay. 

Thank you for review!

> Only minor concern is whether we have to
> apply casts to the results of geteuid() and st.st_uid when used with
> %d format specifier?

I didn't see any complains neither from jprt nor building locally.

uid_t is 4 byte type for both 32bit/64bit so I don't think we need to cast.

-Dmitry


On 2016-08-15 07:23, David Holmes wrote:
> On 12/08/2016 7:04 PM, Dmitry Samersoff wrote:
>> David,
>> 
>> Updated webrev is:
>> 
>> http://cr.openjdk.java.net/~dsamersoff/JDK-8157236/webrev.04/
> 
> Thanks, this looks okay. Only minor concern is whether we have to
> apply casts to the results of geteuid() and st.st_uid when used with
> %d format specifier?
> 
>> Windows is absolutely different story that requires significant
>> efforts to reproduce error conditions and test changes. Also it has
>> nothing to do with ARMv7.
>> 
>> So I would prefer to address windows issues separately either as a
>> part of JDK-8159799 or as a separate CR.
> 
> Ok.
> 
> Thanks, David
> 
>> -Dmitry
>> 
>> On 2016-08-12 03:24, David Holmes wrote:
>>> Hi Dmitry,
>>> 
>>> On 12/08/2016 2:55 AM, Dmitry Samersoff wrote:
>>>> David,
>>>> 
>>>> Please see updated webrev.
>>>> 
>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8157236/webrev.03/
>>>> 
>>>> I didn't touch windows version because it quite different from
>>>> *NIX one.
>>> 
>>> Do we ever see failures on Windows? Is so we should add
>>> diagnostics there too even if they are different to *NIX.
>>> 
>>> I would still like to see what file it is working with. We need
>>> some logging in here:
>>> 
>>> bool AttachListener::is_init_trigger() { if (init_at_startup() ||
>>> is_initialized()) { return false;               // initialized at
>>> startup or already initialized } char fn[PATH_MAX+1]; sprintf(fn,
>>> ".attach_pid%d", os::current_process_id()); int ret; struct
>>> stat64 st; RESTARTABLE(::stat64(fn, &st), ret); if (ret == -1) { 
>>> +    log ("failed to find attach file: %s, trying alternate",
>>> fn) snprintf(fn, sizeof(fn), "%s/.attach_pid%d", 
>>> os::get_temp_directory(), os::current_process_id()); 
>>> RESTARTABLE(::stat64(fn, &st), ret); +    if (ret == -1) { +
>>> log("failed to find attach file: %s", fn); +    } }
>>> 
>>> All failure paths need to show us what it was that failed.
>>> 
>>> typos: trigerred -> triggered
>>> 
>>> Thanks, David
>>> 
>>>> -Dmitry
>>>> 
>>>> On 2016-08-08 02:40, David Holmes wrote:
>>>>> Hi Dmitry,
>>>>> 
>>>>> On 5/08/2016 7:25 PM, Dmitry Samersoff wrote:
>>>>>> Everybody,
>>>>>> 
>>>>>> Please review the fix:
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8157236/webrev.02/
>>>>>>
>>>>>>
>>>>>> 
Problem:
>>>>>> Tests fail intermittently because it can't attach to child
>>>>>> process, these attach failures is hard to debug because
>>>>>> attach framework doesn't provide enough diagnostic
>>>>>> information.
>>>>>> 
>>>>>> Solution:
>>>>>> 
>>>>>> a) Increase attach timeout b) Slightly change attach loop
>>>>>> to save a bit of CPU power. c) Add some logging to attach
>>>>>> listener.
>>>>>> 
>>>>>> It's just a first step in this direction. Complete cleanup
>>>>>> of attach code (remove LinuxThreads support and convert all
>>>>>> printing to UL) is not a goal of this fix - I'll file a
>>>>>> separate CR for it.
>>>>> 
>>>>> I still think you need more logging now to aid in debugging
>>>>> these cases. In particular we want to be able to verify that
>>>>> the path of the attach file is what we expect in all cases ie
>>>>> whether we find  the .attach_pid file in cwd or whether we
>>>>> are looking in temp directory, and whether we ultimately
>>>>> succeed or fail.
>>>>> 
>>>>> Plus whatever you do now should be done consistently for all 
>>>>> platforms.
>>>>> 
>>>>> Thanks, David
>>>>> 
>>>>>> -Dmitry
>>>>>> 
>>>> 
>>>> 
>> 
>> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.