RFR: 8010278 SA: provide mechanism for using an alternative SA debugger back-end.

[Dmitry Samersoff]

Kevin,


* CommandProcessor.java:

"quit" should be volatile.

* HSDB.java:

 argError logic is not clean to me. Is it possible to just do return
after doUsage or use System.exit()?


* HotSpotAgent.java:

1.

You can probably check whether "debugger" variable is already set or not
and get rid of "doAttach" and thus simplify code a bit.

2.

setupDebuggerAlternate doesn't depend to
value of os and cpu, so it might have sense to move this call to upper
level.

672:

Comment is redundant.


* VM.java:

After your changes some code in static initializer
e.g. disableDerivedPrinterTableCheck
is executed ever in case of error.

I'm not sure we should try to recover here - if we can't load
properties, something very bad is happening.


LinuxAddress.java, LinuxOopHandle.java :

What is the reason to make it public?


* ClassDump.java:

Does these changes related to this CR?



On 2013-06-24 15:24, Kevin Walls wrote:
> Thanks Staffan,
> 
> Yes, the GUI does still close and the standalone HSDB ends, without
> forcibly terminating the process if it's initiated by some other app.
> 
> Can I get anybody else interested in reviewing, commenting, etc...?
> 
> Thanks
> Kevin
> 
> 
> On 24/06/13 12:18, Staffan Larsen wrote:
>> On 24 jun 2013, at 11:20, Staffan Larsen<staffan.larsen at oracle.com> 
>> wrote:
>>
>>> On 19 jun 2013, at 14:40, Kevin Walls<kevin.walls at oracle.com>  wrote:
>>>
>>>> Hi Staffan --
>>>>
>>>> And apologies from me for the slow turnaround. 8-)
>>>>
>>>> Thanks for the suggestions, implementing them all.
>>> Thanks.
>>>
>>>> http://cr.openjdk.java.net/~kevinw/8010278/webrev.02/
>>>>
>>>> Also adding the same kind of change to the HSDB tool, a few changes
>>>> there to get the gui closing without using System.exit.
>>> So how do I now exit the HSDB tool? Closing the window won't exit.
>>> File->Exit won't exit. Or did I miss something?
>> I did miss that the workerThread is also terminated in closeUI() and
>> that causes the app to exit.
>>
>> This looks good. Reviewed.
>>
>> /Staffan
>>
>>> /Staffan
>>>
>>>> Additional feedback gratefully received...
>>>>
>>>> Thanks
>>>> Kevin
>>>>
>>>>
>>>> On 05/06/13 12:00, Staffan Larsen wrote:
>>>>> Some comments (sorry it took so long):
>>>>>
>>>>> CLHSDB.java
>>>>> - Can you move the jvmDebugger field to where the other fields are
>>>>> in the class? Make it private, too.
>>>>> - Remove start() and make run() public?
>>>>> - Perhaps improve on the comment in run() saying that either
>>>>> jvmDebugger OR pidText OR {execPath, coreFileName} should be set.
>>>>>
>>>>> HotspotAgent.java
>>>>> - Should sa.altHotSpotAgent be called sa.altDebugger ?
>>>>>
>>>>> Tool.java
>>>>> - Make jvmDebugger private.
>>>>>
>>>>>
>>>>> Regards,
>>>>> /Staffan
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 23 maj 2013, at 15:23, Kevin Walls<kevin.walls at oracle.com>   wrote:
>>>>>
>>>>>> Forgot to mention:
>>>>>>
>>>>>> I moved the ClassDump Tool around a little so it was usable via a
>>>>>> route other than calling main(), and added the constructor that
>>>>>> takes a String for the pkgList, which saves using the system
>>>>>> property to communicate what classes you want to dump
>>>>>> (sun.jvm.hotspot.tools.jcore.PackageNameFilter has that
>>>>>> constructor already).
>>>>>>
>>>>>> Actually, considering the package filter name is always going to
>>>>>> be sun.jvm.hotspot.tools.jcore.PackageNameFilter, let's have that
>>>>>> as a default value when we call getProperty.  Similarly the
>>>>>> getProperty for outputDir, and at that point I stop tweaking.
>>>>>>
>>>>>> A little indenting was off also and I added a comment, so I redid
>>>>>> the webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~kevinw/8010278/webrev.01/
>>>>>>
>>>>>> Thanks
>>>>>> Kevin
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 23/05/13 11:00, Kevin Walls wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> As the Serviceability Agent has been _the_ new and interesting
>>>>>>> way to find things post-mortem in the JVM [1], I'd like to
>>>>>>> propose an update which continues that tradition.
>>>>>>>
>>>>>>> 8010278 SA: provide mechanism for using an alternative SA
>>>>>>> debugger back-end.
>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-8010278
>>>>>>> http://cr.openjdk.java.net/~kevinw/8010278/webrev.00/
>>>>>>>
>>>>>>> This is about making the SA more flexible, so we aren't tied to
>>>>>>> the given native libraries to open cores/memory dumps.  Given
>>>>>>> this change, a 3rd party debugger or tool can interact freely
>>>>>>> with the SA tools (StackTrace, ObjectHistogram, etc...) and
>>>>>>> provide its own backend implementation to actually open a
>>>>>>> core/memory dump.
>>>>>>>
>>>>>>> Primarily for platform-independent core file debugging.  If you
>>>>>>> ever had to open a "foreign" core, find the right hardware,
>>>>>>> etc... this is relevant.   I'm thinking of
>>>>>>> https://java.net/projects/kjdb which can serve as a proof of
>>>>>>> concept.
>>>>>>>
>>>>>>> The changes are:
>>>>>>>
>>>>>>> The main redirection is in HotSpotAgent.java, where we respect a
>>>>>>> property (i.e. -Dsa.altHotSpotAgent=...) to name an alternate
>>>>>>> debugger.
>>>>>>>
>>>>>>> Remove calls to System.exit.
>>>>>>>
>>>>>>> Tool classes (and CLHSDB) should have a constructor that takes a
>>>>>>> JVMDebugger, to remove the assumption that a Tool's JVM will only
>>>>>>> ever contain one debugee.  It doesn't address that VM is a
>>>>>>> singleton and if a tool opens multiple sessions then they would
>>>>>>> need to be from the same JVM version.
>>>>>>>
>>>>>>>
>>>>>>> Thanks
>>>>>>> Kevin
>>>>>>>
>>>>>>> [1]  If you weren't in a circa 1.4.2 demo of the SA when all you
>>>>>>> had previously was a few fragile dbx macros, that got you a few
>>>>>>> very specific details, the night vs. day comparison of no SA vs.
>>>>>>> SA could be missed. 8-)
>>>>>>>
>>>>>>>
> 


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