PING: RFR: JDK-8151815: Could not parse core image with JSnap.

David Holmes david.holmes at oracle.com
Fri Oct 20 05:05:22 UTC 2017


Looks good. (Sorry for the delay.)

Thanks,
David

On 19/10/2017 11:43 PM, Yasumasa Suenaga wrote:
> Sorry, I forgot the fix to use OrderAccess::load_acquire() in 
> PerfMemory::is_initialized().
> I fixed it in new webrev. Could you review again?
> 
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.09/
> 
> 
> Yasumasa
> 
> 
> On 2017/10/19 21:24, David Holmes wrote:
>> On 19/10/2017 9:44 PM, Yasumasa Suenaga wrote:
>>> Hi,
>>>
>>>> I suggest we leave the volatile off for now and file a RFE to add 
>>>> volatile_static_field support to VMStructs and update later.
>>>
>>> Okay. David or Serguei, could you file it?
>>>
>>>
>>>>> I'd suggest to fall back to your previous approach as 
>>>>> synchronization was not there
>>>>> in the first place, and it is not a part of the original issue you 
>>>>> are trying to fix
>>>>> (if David or anyone else does not a simple solution).
>>>
>>>> I don't think trying to introduce locking would be a good idea as it 
>>>> would likely lead to deadlocks when a crash occurs. This could also 
>>>> be investigated as a future RFE if desired.
>>>
>>> Sorry, I have mistake the spell of "usable".
>>> I've fixed it in new webrev:
>>>
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.08/
>>>
>>> Can I list David and Serguei as Reviewer?
>>> I will send a changeset to Serguei if it can.
>>
>> Yes.
>>
>> Thanks,
>> David
>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2017/10/19 19:41, David Holmes wrote:
>>>> Hi Serguei, Yasumasa,
>>>>
>>>> I suggest we leave the volatile off for now and file a RFE to add 
>>>> volatile_static_field support to VMStructs and update later.
>>>>
>>>> I don't think trying to introduce locking would be a good idea as it 
>>>> would likely lead to deadlocks when a crash occurs. This could also 
>>>> be investigated as a future RFE if desired.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 19/10/2017 7:37 PM, serguei.spitsyn at oracle.com wrote:
>>>>> Hi Yasumasa,
>>>>>
>>>>> I see the problem.
>>>>> As it occurred making these variables volatile is non-trivial.
>>>>> But thank you a lot for trying!
>>>>>
>>>>> I'd suggest to fall back to your previous approach as 
>>>>> synchronization was not there
>>>>> in the first place, and it is not a part of the original issue you 
>>>>> are trying to fix
>>>>> (if David or anyone else does not a simple solution).
>>>>> But let's check if David does not object against it.
>>>>>
>>>>> I will sponsor your fix after you send me a patch.
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>> On 10/18/17 20:21, Yasumasa Suenaga wrote:
>>>>>> Sorry, I have mistake.
>>>>>> But I cannot compile yet:
>>>>>>
>>>>>> diff -r 3e7702cd3f19 src/hotspot/share/runtime/vmStructs.cpp
>>>>>> --- a/src/hotspot/share/runtime/vmStructs.cpp   Thu Sep 07 
>>>>>> 15:40:20 2017 +0200
>>>>>> +++ b/src/hotspot/share/runtime/vmStructs.cpp   Thu Oct 19 
>>>>>> 12:21:11 2017 +0900
>>>>>> @@ -578,7 +578,7 @@
>>>>>>       static_field(PerfMemory, _top, 
>>>>>> char*)                                 \
>>>>>>       static_field(PerfMemory, _capacity, 
>>>>>> size_t)                                \
>>>>>>       static_field(PerfMemory, _prologue, 
>>>>>> PerfDataPrologue*)                     \
>>>>>> -     static_field(PerfMemory, _initialized, 
>>>>>> jint)                                  \
>>>>>> +     static_field(PerfMemory, 
>>>>>> _initialized,                                  volatile 
>>>>>> jint)                            \
>>>>>>
>>>>>> --------------
>>>>>> In file included from 
>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:104:0: 
>>>>>>
>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.hpp:168:58: 
>>>>>> error: invalid conversion from 'volatile void*' to 'void*' 
>>>>>> [-fpermissive]
>>>>>>   { QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, 
>>>>>> &typeName::fieldName },
>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:581:6: 
>>>>>> note: in expansion of macro 'GENERATE_STATIC_VM_STRUCT_ENTRY'
>>>>>>       static_field(PerfMemory, 
>>>>>> _initialized,                                  volatile 
>>>>>> jint)                            \
>>>>>>       ^~~~~~~~~~~~
>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:2934:3: 
>>>>>> note: in expansion of macro 'VM_STRUCTS'
>>>>>>    VM_STRUCTS(GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
>>>>>>    ^
>>>>>> gmake[3]: *** [lib/CompileJvm.gmk:210: 
>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/build/linux-x86_64-normal-server-fastdebug/hotspot/variant-server/libjvm/objs/vmStructs.o] 
>>>>>> Error 1
>>>>>> gmake[2]: *** [make/Main.gmk:266: hotspot-server-libs] Error 2
>>>>>>
>>>>>> ERROR: Build failed for target 'images' in configuration 
>>>>>> 'linux-x86_64-normal-server-fastdebug' (exit code 2)
>>>>>> --------------
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2017/10/19 12:18, Yasumasa Suenaga wrote:
>>>>>>> Hi Serguei,
>>>>>>>
>>>>>>>> Would the below work? :
>>>>>>>>
>>>>>>>>   578      static_field(PerfMemory, _initialized, volatile 
>>>>>>>> jint)                                  \
>>>>>>>>
>>>>>>>> It'd be similar to this non-static case:
>>>>>>>>   362   nonstatic_field(ConstantPoolCacheEntry, 
>>>>>>>> _f1,                                  volatile 
>>>>>>>> Metadata*)                    \
>>>>>>>
>>>>>>> I got error messages as below:
>>>>>>>
>>>>>>> ---------------
>>>>>>> In file included from 
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:104:0: 
>>>>>>>
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:581:39: 
>>>>>>> error: expected unqualified-id before 'volatile'
>>>>>>>        static_field(PerfMemory,         volatile _initialized, 
>>>>>>> jint)                                  \
>>>>>>>                                         ^
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.hpp:168:69: 
>>>>>>> note: in definition of macro 'GENERATE_STATIC_VM_STRUCT_ENTRY'
>>>>>>>    { QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, 
>>>>>>> &typeName::fieldName },
>>>>>>> ^~~~~~~~~
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:2934:3: 
>>>>>>> note: in expansion of macro 'VM_STRUCTS'
>>>>>>>     VM_STRUCTS(GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
>>>>>>>     ^
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:581:39: 
>>>>>>> error: expected '}' before 'volatile'
>>>>>>>        static_field(PerfMemory,         volatile _initialized, 
>>>>>>> jint)                                  \
>>>>>>>                                         ^
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.hpp:168:69: 
>>>>>>> note: in definition of macro 'GENERATE_STATIC_VM_STRUCT_ENTRY'
>>>>>>>    { QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, 
>>>>>>> &typeName::fieldName },
>>>>>>> ^~~~~~~~~
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:2934:3: 
>>>>>>> note: in expansion of macro 'VM_STRUCTS'
>>>>>>>     VM_STRUCTS(GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
>>>>>>>     ^
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:581:39: 
>>>>>>> error: expected '}' before 'volatile'
>>>>>>>        static_field(PerfMemory,         volatile _initialized, 
>>>>>>> jint)                                  \
>>>>>>>                                         ^
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.hpp:168:69: 
>>>>>>> note: in definition of macro 'GENERATE_STATIC_VM_STRUCT_ENTRY'
>>>>>>>    { QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, 
>>>>>>> &typeName::fieldName },
>>>>>>> ^~~~~~~~~
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:2934:3: 
>>>>>>> note: in expansion of macro 'VM_STRUCTS'
>>>>>>>     VM_STRUCTS(GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
>>>>>>>     ^
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.hpp:168:79: 
>>>>>>> error: expected declaration before '}' token
>>>>>>>    { QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, 
>>>>>>> &typeName::fieldName },
>>>>>>> ^
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:581:6: 
>>>>>>> note: in expansion of macro 'GENERATE_STATIC_VM_STRUCT_ENTRY'
>>>>>>>        static_field(PerfMemory,         volatile _initialized, 
>>>>>>> jint)                                  \
>>>>>>>        ^~~~~~~~~~~~
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/src/hotspot/share/runtime/vmStructs.cpp:2934:3: 
>>>>>>> note: in expansion of macro 'VM_STRUCTS'
>>>>>>>     VM_STRUCTS(GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
>>>>>>>     ^
>>>>>>> gmake[3]: *** [lib/CompileJvm.gmk:210: 
>>>>>>> /home/ysuenaga/OpenJDK/jdk10-hs/build/linux-x86_64-normal-server-fastdebug/hotspot/variant-server/libjvm/objs/vmStructs.o] 
>>>>>>> Error 1
>>>>>>> gmake[2]: *** [make/Main.gmk:266: hotspot-server-libs] Error 2
>>>>>>>
>>>>>>> ERROR: Build failed for target 'images' in configuration 
>>>>>>> 'linux-x86_64-normal-server-fastdebug' (exit code 2)
>>>>>>> ---------------
>>>>>>>
>>>>>>>
>>>>>>> I changed as below:
>>>>>>> ---------------
>>>>>>> diff -r 3e7702cd3f19 src/hotspot/share/runtime/perfMemory.cpp
>>>>>>> --- a/src/hotspot/share/runtime/perfMemory.cpp  Thu Sep 07 
>>>>>>> 15:40:20 2017 +0200
>>>>>>> +++ b/src/hotspot/share/runtime/perfMemory.cpp  Thu Oct 19 
>>>>>>> 12:15:30 2017 +0900
>>>>>>> @@ -51,8 +51,9 @@
>>>>>>>   char*                    PerfMemory::_end = NULL;
>>>>>>>   char*                    PerfMemory::_top = NULL;
>>>>>>>   size_t                   PerfMemory::_capacity = 0;
>>>>>>> -jint                     PerfMemory::_initialized = false;
>>>>>>> +volatile jint            PerfMemory::_initialized = 0;
>>>>>>>   PerfDataPrologue*        PerfMemory::_prologue = NULL;
>>>>>>> +volatile bool            PerfMemory::_destroyed = false;
>>>>>>>
>>>>>>> --- a/src/hotspot/share/runtime/perfMemory.hpp  Thu Sep 07 
>>>>>>> 15:40:20 2017 +0200
>>>>>>> +++ b/src/hotspot/share/runtime/perfMemory.hpp  Thu Oct 19 
>>>>>>> 12:15:30 2017 +0900
>>>>>>> @@ -113,13 +113,15 @@
>>>>>>>    */
>>>>>>>   class PerfMemory : AllStatic {
>>>>>>>       friend class VMStructs;
>>>>>>> +    friend class PerfMemoryTest;
>>>>>>>     private:
>>>>>>>       static char*  _start;
>>>>>>>       static char*  _end;
>>>>>>>       static char*  _top;
>>>>>>>       static size_t _capacity;
>>>>>>>       static PerfDataPrologue*  _prologue;
>>>>>>> -    static jint   _initialized;
>>>>>>> +    static volatile jint      _initialized;
>>>>>>> +    static volatile bool      _destroyed;
>>>>>>>
>>>>>>> diff -r 3e7702cd3f19 src/hotspot/share/runtime/vmStructs.cpp
>>>>>>> --- a/src/hotspot/share/runtime/vmStructs.cpp   Thu Sep 07 
>>>>>>> 15:40:20 2017 +0200
>>>>>>> +++ b/src/hotspot/share/runtime/vmStructs.cpp   Thu Oct 19 
>>>>>>> 12:15:30 2017 +0900
>>>>>>> @@ -578,7 +578,7 @@
>>>>>>>        static_field(PerfMemory, _top, 
>>>>>>> char*)                                 \
>>>>>>>        static_field(PerfMemory, _capacity, 
>>>>>>> size_t)                                \
>>>>>>>        static_field(PerfMemory, _prologue, 
>>>>>>> PerfDataPrologue*)                     \
>>>>>>> -     static_field(PerfMemory, _initialized, 
>>>>>>> jint)                                  \
>>>>>>> +     static_field(PerfMemory,         volatile _initialized, 
>>>>>>> jint)                                  \
>>>>>>> ---------------
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2017/10/19 6:18, serguei.spitsyn at oracle.com wrote:
>>>>>>>> On 10/18/17 06:51, Yasumasa Suenaga wrote:
>>>>>>>>> Hi David, Serguei,
>>>>>>>>>
>>>>>>>>>> because as soon as we have checked is_usable() and abort 
>>>>>>>>>> happening in another thread may have changed that by calling 
>>>>>>>>>> destroy.
>>>>>>>>>>
>>>>>>>>>> This code is basically broken if we hit an abort path instead 
>>>>>>>>>> of a normal VM shutdown.
>>>>>>>>>
>>>>>>>>> Can we use MutexLocker for initialize() and destroy() ?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I've tried to fix about your comments, but I have an issue 
>>>>>>>>> about volatile.
>>>>>>>>> PerfMemory.java depends on PerfMemory::_initialized. However 
>>>>>>>>> VMStructs cannot handle static volatile variables.
>>>>>>>>> I think two approaches as below:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>   1. Remove _initialized check from PerfMemory.java
>>>>>>>>>      SA will throw UnmappedAddressException if JSnap try to 
>>>>>>>>> access invalid address including uninitialized memory.
>>>>>>>>>
>>>>>>>>>   2. Add static volatile support to VMStructs
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Which should we do?
>>>>>>>>> 1. is easy to fix. But 2. might be right way...
>>>>>>>>
>>>>>>>> Would the below work? :
>>>>>>>>
>>>>>>>>   578      static_field(PerfMemory, _initialized, volatile 
>>>>>>>> jint)                                  \
>>>>>>>>
>>>>>>>> It'd be similar to this non-static case:
>>>>>>>>   362   nonstatic_field(ConstantPoolCacheEntry, 
>>>>>>>> _f1,                                  volatile 
>>>>>>>> Metadata*)                    \
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Serguei
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2017/10/18 21:34, David Holmes wrote:
>>>>>>>>>> Just to clarify ...
>>>>>>>>>>
>>>>>>>>>> On 18/10/2017 10:28 PM, David Holmes wrote:
>>>>>>>>>>> On 18/10/2017 8:26 PM, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>
>>>>>>>>>>>> Thank you for jumping to this review and helping Yasumasa to 
>>>>>>>>>>>> sort it out!
>>>>>>>>>>>> I've just discovered that this issue was already on the 
>>>>>>>>>>>> table for several months without a significant progress.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 10/18/17 02:48, David Holmes wrote:
>>>>>>>>>>>>> Hi Serguei
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 18/10/2017 7:25 PM, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Sorry for a quite late participation.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I looked at the previous webrevs and think that this one 
>>>>>>>>>>>>>> is much better.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Some concern is if we need any kind of synchronization 
>>>>>>>>>>>>>> here, e.g. CAS.
>>>>>>>>>>>>>> But it depends on the PerfMemory class usage.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Should we make the static variables '_initialized' and 
>>>>>>>>>>>>>> '_destroyed' volatile?
>>>>>>>>>>>>>
>>>>>>>>>>>>> For good measure - yes.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Also, the '_initialized' is set to 1 with:
>>>>>>>>>>>>>>     159 OrderAccess::release_store(&_initialized, 1);
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Should we do the same to set the '_destroyed'?:
>>>>>>>>>>>>>> 200 _destroyed = true;
>>>>>>>>>>>>>
>>>>>>>>>>>>> There is a benign initialization race but we need the 
>>>>>>>>>>>>> release_store to ensure all the data fields can be read if 
>>>>>>>>>>>>> _initialized is seen as true. But what is missing is a 
>>>>>>>>>>>>> load_acquire() in is_initialized() to ensure we synchronize 
>>>>>>>>>>>>> with that store!
>>>>>>>>>>>>
>>>>>>>>>>>> Yes, I noticed that the load_acquire() is missed. :|
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> There is also a potential for a destruction race (if 
>>>>>>>>>>>>> multiple aborts happens concurrently in different threads) 
>>>>>>>>>>>>> but that also seems benign. In this case there is no data 
>>>>>>>>>>>>> being set so the store to _destroyed does not need to be a 
>>>>>>>>>>>>> release_store.
>>>>>>>>>>>>
>>>>>>>>>>>> I'm not convinced yet this is benign as the 
>>>>>>>>>>>> PerfMemory::destroy() has this call:
>>>>>>>>>>>>    197 delete_memory_region();
>>>>>>>>>>>
>>>>>>>>>>> Yes though most of its work ends up being no-ops.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Now, I started thinking about the asserts that call the 
>>>>>>>>>>>> is_useable().
>>>>>>>>>>>> Should they be returns instead?
>>>>>>>>>>>
>>>>>>>>>>> I think this is a somewhat confused chunk of code. It's only 
>>>>>>>>>>> fractionally thread-safe yet once in use could be in use 
>>>>>>>>>>> concurrently with an aborting thread that calls destroy(). I 
>>>>>>>>>>> don't think there is any simple fix for this. If we're in the 
>>>>>>>>>>> process of crashing does it really matter if we trigger a 
>>>>>>>>>>> secondary crash due to this?
>>>>>>>>>>
>>>>>>>>>> It doesn't matter if we do:
>>>>>>>>>>
>>>>>>>>>> assert(is_usable(),...);
>>>>>>>>>> // continue
>>>>>>>>>>
>>>>>>>>>> or
>>>>>>>>>>
>>>>>>>>>> if (!is_usable()) return;
>>>>>>>>>> // continue
>>>>>>>>>>
>>>>>>>>>> because as soon as we have checked is_usable() and abort 
>>>>>>>>>> happening in another thread may have changed that by calling 
>>>>>>>>>> destroy.
>>>>>>>>>>
>>>>>>>>>> This code is basically broken if we hit an abort path instead 
>>>>>>>>>> of a normal VM shutdown.
>>>>>>>>>>
>>>>>>>>>> David
>>>>>>>>>> -----
>>>>>>>>>>
>>>>>>>>>>> The problems with this code go way beyond what Yasumasa is 
>>>>>>>>>>> trying to address with the JSnap problem and I would not want 
>>>>>>>>>>> to put it back on him to try and come up with an overall 
>>>>>>>>>>> solution.
>>>>>>>>>>>
>>>>>>>>>>>> Then the is_destroyed() would better to have the 
>>>>>>>>>>>> load_acquire().
>>>>>>>>>>>
>>>>>>>>>>> You could add a load_acquire and do the store_release. It 
>>>>>>>>>>> certainly would not hurt, but I don't think it would actually 
>>>>>>>>>>> benefit anything either.
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>>> Just interested to know what do you think on this.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Serguei
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>> David
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 10/18/17 00:39, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thank you for your comment.
>>>>>>>>>>>>>>> I uploaded new webrev:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.07/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Serguei, please comment about this :-)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 2017-10-18 16:09 GMT+09:00 David 
>>>>>>>>>>>>>>> Holmes<david.holmes at oracle.com>:
>>>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 18/10/2017 4:34 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I don't think we need the extra fields, just ensure 
>>>>>>>>>>>>>>>>>> the existing ones
>>>>>>>>>>>>>>>>>> can't
>>>>>>>>>>>>>>>>>> be accessed (other than by the tools) after destroy is 
>>>>>>>>>>>>>>>>>> called.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I've added PerfMemory::is_useable() to check whether we 
>>>>>>>>>>>>>>>>> can access to
>>>>>>>>>>>>>>>>> PerfMemory.
>>>>>>>>>>>>>>>>> I think this webrev prevent to access to PerfMemory 
>>>>>>>>>>>>>>>>> after destroy() call.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.06/ 
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> This:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>    90 void PerfMemory::initialize() {
>>>>>>>>>>>>>>>>    91
>>>>>>>>>>>>>>>>    92   if (_prologue != NULL)
>>>>>>>>>>>>>>>>    93     // initialization already performed
>>>>>>>>>>>>>>>>    94     return;
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> shouldn't check _prologue, but is_initialized().
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>   213   assert(is_useable(), "called before 
>>>>>>>>>>>>>>>> initialization");
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> -> "called before init or after destroy"
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Could add a similar assert in PerfMemory::mark_updated().
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Let's see what Serguei thinks. :)
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 2017-10-18 13:44 GMT+09:00 David 
>>>>>>>>>>>>>>>>> Holmes<david.holmes at oracle.com>:
>>>>>>>>>>>>>>>>>> On 18/10/2017 2:27 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> 2017-10-18 12:55 GMT+09:00 David 
>>>>>>>>>>>>>>>>>>> Holmes<david.holmes at oracle.com>:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 18/10/2017 12:37 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> With your changes you no longer null out _prologue 
>>>>>>>>>>>>>>>>>>>>>> so the assertion
>>>>>>>>>>>>>>>>>>>>>> would
>>>>>>>>>>>>>>>>>>>>>> now not fail and we'd proceed to access the 
>>>>>>>>>>>>>>>>>>>>>> deleted memory region!
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On Linux, PerfMemory::delete_memory_region() does 
>>>>>>>>>>>>>>>>>>>>> not call munmap()
>>>>>>>>>>>>>>>>>>>>> for PerfMemory.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Perhaps not but there are still other actions that 
>>>>>>>>>>>>>>>>>>>> happen and the point
>>>>>>>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>>>> we should not be able to continue to use PerfMemory 
>>>>>>>>>>>>>>>>>>>> once it has been
>>>>>>>>>>>>>>>>>>>> destroyed (even if the destruction is only logical).
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I received same comment from Dmitry in the past, but 
>>>>>>>>>>>>>>>>>>> we couldn't
>>>>>>>>>>>>>>>>>>> decide how should we do.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-May/019728.html 
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> In that discussion, I uploaded another webrev which 
>>>>>>>>>>>>>>>>>>> adds other fields
>>>>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>>>> JSnap.
>>>>>>>>>>>>>>>>>>> Is it suitable?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/ 
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I don't think we need the extra fields, just ensure 
>>>>>>>>>>>>>>>>>> the existing ones
>>>>>>>>>>>>>>>>>> can't
>>>>>>>>>>>>>>>>>> be accessed (other than by the tools) after destroy is 
>>>>>>>>>>>>>>>>>> called.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> I'm unclear why you no longer clear all the fields 
>>>>>>>>>>>>>>>>>>>>>> set during
>>>>>>>>>>>>>>>>>>>>>> initialization?
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> PerfMemory.java in jdk.hotspot.agent needs these 
>>>>>>>>>>>>>>>>>>>>> field values.
>>>>>>>>>>>>>>>>>>>>> `jhsdb jsnap --core` is failed if they are cleared.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> I'm not familiar with these tools. When do we 
>>>>>>>>>>>>>>>>>>>> produce a core file after
>>>>>>>>>>>>>>>>>>>> calling PerfMemory::destroy ?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> PerfMemory::destroy() is called before aborting.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Ah - right. I assume we need to close off the perfdata 
>>>>>>>>>>>>>>>>>> file before we
>>>>>>>>>>>>>>>>>> abort.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> -----------------------
>>>>>>>>>>>>>>>>>>> #0  perfMemory_exit ()
>>>>>>>>>>>>>>>>>>>        at
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/share/vm/runtime/perfMemory.cpp:80 
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> #1  0x00007f99b091c949 in os::shutdown ()
>>>>>>>>>>>>>>>>>>>        at
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/os/linux/vm/os_linux.cpp:1483 
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> #2  0x00007f99b091c980 in os::abort 
>>>>>>>>>>>>>>>>>>> (dump_core=<optimized out>)
>>>>>>>>>>>>>>>>>>>        at
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/os/linux/vm/os_linux.cpp:1503 
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> #3  0x00007f99b0b689c3 in VMError::report_and_die (
>>>>>>>>>>>>>>>>>>>        this=this at entry=0x7ffcacf40b50)
>>>>>>>>>>>>>>>>>>>        at
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/share/vm/utilities/vmError.cpp:1060 
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> #4  0x00007f99b0926f04 in JVM_handle_linux_signal 
>>>>>>>>>>>>>>>>>>> (sig=sig at entry=11,
>>>>>>>>>>>>>>>>>>>        info=info at entry=0x7ffcacf40df0,
>>>>>>>>>>>>>>>>>>> ucVoid=ucVoid at entry=0x7ffcacf40cc0,
>>>>>>>>>>>>>>>>>>> abort_if_unrecognized=abort_if_unrecognized at entry=1)
>>>>>>>>>>>>>>>>>>>        at
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/os_cpu/linux_x86/vm/os_linux_x86.cpp:541 
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> -----------------------
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> But it seems to me that there are various checks of
>>>>>>>>>>>>>>>>>>>>>> _prologue that should really be checking 
>>>>>>>>>>>>>>>>>>>>>> is_initialized() and/or
>>>>>>>>>>>>>>>>>>>>>> is_destroyed() as a guard.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Should I change all assertions for _prologue?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Assertions and direct guards. Checking _prologue is 
>>>>>>>>>>>>>>>>>>>> a placeholder for
>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>> real check.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 2017-10-18 10:53 GMT+09:00 David 
>>>>>>>>>>>>>>>>>>>>> Holmes<david.holmes at oracle.com>:
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> By chance we ran into this bug which I analysed 
>>>>>>>>>>>>>>>>>>>>>> yesterday:
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8189390
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> We hit the assertion:
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> #  Internal Error
>>>>>>>>>>>>>>>>>>>>>> (/open/src/hotspot/share/runtime/perfMemory.cpp:216),
>>>>>>>>>>>>>>>>>>>>>> pid=17874, tid=17875
>>>>>>>>>>>>>>>>>>>>>> #  assert(_prologue != __null) failed: called 
>>>>>>>>>>>>>>>>>>>>>> before initialization
>>>>>>>>>>>>>>>>>>>>>> #
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> which is misleading because it can fail if called 
>>>>>>>>>>>>>>>>>>>>>> before
>>>>>>>>>>>>>>>>>>>>>> initialization,
>>>>>>>>>>>>>>>>>>>>>> or
>>>>>>>>>>>>>>>>>>>>>> after PerfMemory::destroy has been called.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> With your changes you no longer null out _prologue 
>>>>>>>>>>>>>>>>>>>>>> so the assertion
>>>>>>>>>>>>>>>>>>>>>> would
>>>>>>>>>>>>>>>>>>>>>> now not fail and we'd proceed to access the 
>>>>>>>>>>>>>>>>>>>>>> deleted memory region!
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> I'm unclear why you no longer clear all the fields 
>>>>>>>>>>>>>>>>>>>>>> set during
>>>>>>>>>>>>>>>>>>>>>> initialization? But it seems to me that there are 
>>>>>>>>>>>>>>>>>>>>>> various checks of
>>>>>>>>>>>>>>>>>>>>>> _prologue that should really be checking 
>>>>>>>>>>>>>>>>>>>>>> is_initialized() and/or
>>>>>>>>>>>>>>>>>>>>>> is_destroyed() as a guard.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> On 16/10/2017 11:25 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> PING:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.05/ 
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> On 2017/10/03 13:18, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> I added gtest unit test case for this change in 
>>>>>>>>>>>>>>>>>>>>>>>> new webrev:
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.05/ 
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> 2017-09-27 0:01 GMT+09:00 Yasumasa 
>>>>>>>>>>>>>>>>>>>>>>>> Suenaga<yasuenag at gmail.com>:
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I uploaded new webrev to be adapted to jdk10/hs:
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.04/ 
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> On 2017/09/21 7:45, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> PING:
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Have you checked this issue?
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.03/ 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> On 2017/07/01 23:43, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> PING:
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> Have you checked this issue?
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> On 2017/06/13 14:10, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> I want to discuss about JDK-8151815: Could 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> not parse core image
>>>>>>>>>>>>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>>>>>>>>>>>>> JSnap.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> In last year, I found JSnap cannot parse 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> coredump and I've sent
>>>>>>>>>>>>>>>>>>>>>>>>>>>> review
>>>>>>>>>>>>>>>>>>>>>>>>>>>> request for it as JDK-8151815. However it 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> has not been reviewed
>>>>>>>>>>>>>>>>>>>>>>>>>>>> yet
>>>>>>>>>>>>>>>>>>>>>>>>>>>> [1].
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> We've discussed about safety implementation, 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> but we could not
>>>>>>>>>>>>>>>>>>>>>>>>>>>> get
>>>>>>>>>>>>>>>>>>>>>>>>>>>> consensus.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> IMHO all SA tools should be handled java 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> processes and core
>>>>>>>>>>>>>>>>>>>>>>>>>>>> images,
>>>>>>>>>>>>>>>>>>>>>>>>>>>> and PerfCounter value is useful. So I fix 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> this issue.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> I uploaded new webrev for this issue. I 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> think this patch is
>>>>>>>>>>>>>>>>>>>>>>>>>>>> safety
>>>>>>>>>>>>>>>>>>>>>>>>>>>> because new flag PerfMemory::_destroyed 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> guards double free, and
>>>>>>>>>>>>>>>>>>>>>>>>>>>> all
>>>>>>>>>>>>>>>>>>>>>>>>>>>> members in PerfMemory is accessible (they 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> are not munmap'ed)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.03/ 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Can you cooperate?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-April/019480.html 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>
>>>>>


More information about the serviceability-dev mailing list