On the 0x25B day of Apache Harmony Ilia A. Leviev wrote:
> Hi,
> Recently I have started to use Thread Checker tool for finding unsafe
> thread access places in VM.
> First I have tried to check VM that run simple application such HWA.
> The tool shows that there is thread unsafe access to some of
> VM_Statistics fields that result in race condition.
JIT profilers and Jitrino.OPT instruction profiler are thread-unsafe
too. But this is a *feature*. We do not need precise data here at a
cost of extra synchronisations.
Same arguments may be applied to vm stats...
> \vm\vmcore\include\ vm_stats.h
>
> class VM_Statistics
> {
> public:
>
> uint64 num_free_local_called;
> uint64 num_jni_handles_wasted_refs;
> uint64 num_free_local_called_free;
> uint64 num_local_jni_handles;
> uint64 num_jni_handles_freed;
>
> ~~~~~~~~~~~~~~~~~~~~~
>
> The problem occurred during execution of free_local_object_handles2
> function
> from \vm\vmcore\src\object\object_handles.cpp.
> The function increment fields of type uint64.
>
> ////////////////////////////////////////////////////////
> void free_local_object_handles2(ObjectHandles* head)
> {
> ObjectHandlesNew* h = (ObjectHandlesNew*)head;
> assert(h);
> #ifdef VM_STATS
> VM_Statistics::get_vm_stats().num_free_local_called++;// race
> condition
> if(h->next != NULL)
> VM_Statistics::get_vm_stats().num_free_local_called_free++;//
> race condition
> #endif //VM_STATS
> while(h->next) {
> #ifdef VM_STATS
> unsigned size = h->size;
> VM_Statistics::get_vm_stats().num_local_jni_handles += size; //
> race condition
> VM_Statistics::get_vm_stats().num_jni_handles_freed++; // race
> condition
> VM_Statistics::get_vm_stats().num_jni_handles_wasted_refs +=
> (h->capacity - size);// race condition
> #endif //VM_STATS
> ObjectHandlesNew* next = h->next;
> STD_FREE(h);
> h = next;
> }
> #ifdef VM_STATS
> VM_Statistics::get_vm_stats().num_jni_handles_wasted_refs +=
> (h->capacity - h->size); // race condition
> #endif //VM_STATS
> }
> ////////////////////////////////////////////////////////
>
> There are several solutions:
> 1) remove and do not use statistics at all, if anybody not needs it.
> 2) make this function synchronized using lock-unlock functions from
> lock_manager,
> 3) make increment by atomic APR function such apr_atomic_inc32(
> volatile apr_uint32_t * mem ),
> but to use it we need to change the type of the fields from uint64
> to volatile uint32.
>
> What solution will be more acceptable?
>
>
> Best regards,
> Leviev Ilya
> Intel Java & XML Engineering
>
--
Egor Pasko
