Ticket #4231 (assigned defect)
crash with many threads in java
| Reported by: | nbrachet | Owned by: | unicoletti |
|---|---|---|---|
| Priority: | normal | Milestone: | 6.0.2 release |
| Component: | MapScript | Version: | unspecified |
| Severity: | normal | Keywords: | mapscript java thread |
| Cc: | sdlime, tamas |
Description
How to reproduce:
Modify MapThread.java to remove the call to System.gc(). For this problem to reproduce one thread has to delete the mapObj object, while simultaneously another one deletes a layerObj object (from the same mapObj object), which happens when garbage collection runs.
Run with many threads for a while... because this is a threading issue it can work for a while without problem. In our testing we would crash within a half hour on a 8-core machine running 20 threads with JRE 1.6.
Analysis:
Reference counting isn't thread-safe.
layerObj objects obtained from a call to mapObj.get(int) are really shared between the layerObj and the mapObj objects. When garbage collection runs it may actually delete the layerObj object simultaneously from 2 different threads: one deleting the layerObj, the other one deleting the mapObj. freeLayer() calls MS_REFCNT_DECR_IS_NOT_ZERO(), itself implemented in terms of MS_REFCNT_DECR() which is a simple macro around operator++... not thread-safe. In some (rare) case both threads think the ref counter went to zero because of their own call to MS_REFCNT_DECR and they end up both calling free() causing the crash.
Solution:
A simple solution is to use atomic int operations, __sync_fetch_and_add/__sync_sub_and_fetch for GCC, InterlockedIncrement/InterlockedDecrement on WIN32, etc... Attached is a patch against 6.0.2 that implements this change for GCC.
