request for review: 6436034: Instance filter doesn't filter event if it occurs in native method

Keith McGuigan keith.mcguigan at oracle.com
Tue Dec 14 06:50:56 PST 2010


On Dec 14, 2010, at 9:38 AM, Daniel D. Daugherty wrote:

> On 12/14/2010 3:23 AM, Alan Bateman wrote:
>> Keith McGuigan wrote:
>>>
>>> Hello,
>>>
>>> This is the JDK-side of the fix for instance filters that uses the  
>>> new GetLocalInstance JVMTI function.
>>>
>>> http://cr.openjdk.java.net/~kamg/6436034/webrev.00/
>>>
>>> Reviews appreciated, thanks!
>>>
>>> -- 
>>> - Keith
>> The update to jvmti.h looks good to me.
>>
>> It's not clear to me that eventFilter.c needs to check the jvmti  
>> version. As this is jdk7 then the JDWP agent will be compiled  
>> against the updated jvmti.h and so the version check in  
>> Agent_OnLoad (debugInit.c) should mean that the agent will refuse  
>> to start if someone were to attempt to use with an older VM.
>
> I had completely forgotten about the version check in Agent_OnLoad.
> Being able to drop an older VM into JDK7 for triage purposes is
> always useful so now I'm wondering if we should add a command line
> option to bypass that version check? Default behavior would be to
> print the detailed error message. Optional behavior would be for
> the message to be a warning and the agent to continue on.
>
> Keith, what do you think?

Yeah I missed that check too.  I'm very much in favor of being able to  
use an older (or newer) JVM in the JDK7 image.  Triage is one  
important reason but another is the non-synchronous putback schedules  
of the jdk and hotspot workspaces.  There will be a period of QA time  
when the JDK uses an older VM (or vice-versa) just because of the way  
our process works.

I think the jdk code should probe the JVMTI version and use whatever  
is available.  I'm in favor of modifying the code in debugInit.c to  
verify only 1.1 and then dynamically probe before using any 1.2  
features (and so on in the future).

--
- Keith


More information about the serviceability-dev mailing list