code review for JVM/TI DynamicCodeGenerated event fix (7039447)

Karen Kinnear karen.kinnear at oracle.com
Thu Apr 28 07:06:37 PDT 2011


Sorry, I didn't know you were going to redo this. I too have memories
of os::free of null having issues. And I also have memories of code checking
tools complaining about calls to os::free of null.

So, if you could either go back to the way it was, or only call
os::free if it is not null I would be much more comfortable.

thanks,
Karen

On 04/28/11 09:52, Daniel D. Daugherty wrote:
> On 4/28/2011 7:37 AM, Daniel D. Daugherty wrote:
>> I'm going to take another look at what I can do to clean this
>> up a bit...
>
> This version should address your concerns and Dmitry's also.
> Especially since Dmitry suggested the specific code in the
> post_dynamic_code_generated_internal() call. I just added
> the comment and reformatted it a bit...
>
> Dan
>
>
> --- a/src/share/vm/prims/jvmtiImpl.cpp  Sat Apr 23 00:33:38 2011 -0400
> +++ b/src/share/vm/prims/jvmtiImpl.cpp  Thu Apr 28 07:44:01 2011 -0600
> @@ -38,6 +38,7 @@
> #include "runtime/handles.inline.hpp"
> #include "runtime/interfaceSupport.hpp"
> #include "runtime/javaCalls.hpp"
> +#include "runtime/os.hpp"
> #include "runtime/serviceThread.hpp"
> #include "runtime/signature.hpp"
> #include "runtime/vframe.hpp"
> @@ -939,10 +940,15 @@
>   nmethodLocker::lock_nmethod(nm, true /* zombie_ok */);
>   return event;
> }
> +
> JvmtiDeferredEvent JvmtiDeferredEvent::dynamic_code_generated_event(
>       const char* name, const void* code_begin, const void* code_end) {
>   JvmtiDeferredEvent event = 
> JvmtiDeferredEvent(TYPE_DYNAMIC_CODE_GENERATED);
> -  event._event_data.dynamic_code_generated.name = name;
> +  // Need to make a copy of the name since we don't know how long
> +  // the event poster will keep it around after we enqueue the
> +  // deferred event and return. strdup() failure is handled in
> +  // the post() routine below.
> +  event._event_data.dynamic_code_generated.name = os::strdup(name);
>   event._event_data.dynamic_code_generated.code_begin = code_begin;
>   event._event_data.dynamic_code_generated.code_end = code_end;
>   return event;
> @@ -968,12 +974,17 @@
>       nmethodLocker::unlock_nmethod(nm);
>       break;
>     }
> -    case TYPE_DYNAMIC_CODE_GENERATED:
> +    case TYPE_DYNAMIC_CODE_GENERATED: {
>       JvmtiExport::post_dynamic_code_generated_internal(
> -        _event_data.dynamic_code_generated.name,
> +        // if strdup failed give the event a default name
> +        (_event_data.dynamic_code_generated.name == NULL)
> +          ? "unknown_code" : _event_data.dynamic_code_generated.name,
>         _event_data.dynamic_code_generated.code_begin,
>         _event_data.dynamic_code_generated.code_end);
> +      // release our copy
> +      os::free(_event_data.dynamic_code_generated.name);
>       break;
> +    }
>     default:
>       ShouldNotReachHere();
>   }
>



More information about the serviceability-dev mailing list