Prepared by: | Volker H. Simonis on Wed Dec 1 19:19:39 CET 2010 |
---|---|
Workspace: | /share/software/Java/OpenJDK/hotspot |
Summary of changes: | 12 lines changed: 9 ins; 0 del; 3 mod; 1240 unchg |
Patch of changes: | 6704010.patch |
Author comments: |
This problem occurs very rarely and it is therefore hardly possible to reliably reproduce it. However after looking at it for a while I think I found the cause of the problem:
The SignatureHandlerLibrary class uses two static GrowableArrays, '_handlers' and '_fingerprints', to store method fingerprints along with their corresponding signature handlers. But GrowableArrays are are NOT synchronized in any way if accessed from multiple threads. To avoid races it is therefore necessary to synchronize different threads which access the same GrowableArray. This is done for most of the code in 'SignatureHandlerLibrary::add()' which is protected by 'MutexLocker mu(SignatureHandlerLibrary_lock)'. However the assertion at the end of the method is outside the scope of this MutexLocker which can lead to an illegal view on the corresponding GrowableArray data structures. Here'S what may happen in detail:
Fixing this can be easily done by enclosing the assertion into a region which is protected by the same Mutex like the main body of 'SignatureHandlerLibrary::add()'. As I wrote, I couldn't reproduce this in the HostSpot, so no regression test. I was however able to reproduce the described scenario with a standalone GrowableArray class and a small test program consisting of one writer thread which appends a constant value to a GrowableArray and ten reader threads which are constantly trying to find that special value in the array on a dual core x86_64 Linux box. As a last side note, the problem could also be made even more unlikely, by changing the implementation of 'GrowableArray::grow()' from: templateto: template On the other hand that's still not 100% safe ('_data' is not volatile, other memory models, ..) and I think GrowableArray is definitely not intended for parallel access and therefore such a change may unnecessarily hide other problems where this precondition is violated. So maybe nulling out the '_data' array before freeing it may be a good idea in the debug build to catch such errors! |
Bug id: | 6704010 Internal Error (src/share/vm/interpreter/interpreterRuntime.cpp:1106) |
Legend: |
Modified file Deleted file New file |
Cdiffs
Udiffs
Sdiffs
Frames
Old
New
Patch
Raw
src/share/vm/interpreter/interpreterRuntime.cpp
12 lines changed: 9 ins; 0 del; 3 mod; 1240 unchg
This code review page was prepared using /usr/local/bin/webrev (vers 23.18-hg).