Bug #6322
closedEnsure lock is obtained prior to calls to Hazelcast IMap.put()
0%
Description
When modifying the system metadata table, we call systemMetadataMap.put(). since this is a distributed map in the HZ cluster, we need to call lock() first on the pid. This particularly needs to be fixed in DocumentImpl, but we should audit this call in other classes to make sure we lock and unlock correctly.
Updated by ben leinfelder almost 11 years ago
I see quite a few hzSystemMetadata.put() calls without locks. And there are some comments like this:
// note: the calling subclass handles the map hazelcast lock/unlock
So I take it the thread that locks the pid has control of it, and any calls made further down the chain still have that lock? If that's the case, I think we have to be careful with our locking so that calls down the callstack don't try to lock a pid that was locked earlier in the stack...
Updated by Matt Jones almost 11 years ago
It seems to me that we should protect against this by creating a wrapper function rather than calling hz.put() directly, where the wrapper function would 1) first check to see if the caller has the lock, and if not, tries to get a lock, and once it has a lock, then calls hz.put() for the caller. Then you wouldn't have to worry about who has the lock as much. We'd have to watch for deadlock scenarios and have a suitable lock release strategy, but this seems like it would be more reliable than just hoping that subclasses handle locks properly.
Updated by ben leinfelder almost 11 years ago
- Target version changed from 2.4.0 to 2.5.0
Updated by ben leinfelder about 9 years ago
Do we have an example of something that is not working correctly because of locks or lack of locks in our code?
Updated by Chris Jones about 9 years ago
- Status changed from New to Rejected
Ben pointed out that we call lock() in the calling methods that invoke DocumentImpl methods (like CNodeService.create()). So, with that in mind, we don't need to lock pids during DocumentImpl calls. I'll reject this ticket.