Bug #6322
closed
Ensure lock is obtained prior to calls to Hazelcast IMap.put()
Added by Chris Jones over 10 years ago.
Updated over 8 years ago.
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.
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...
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.
- Target version changed from 2.4.0 to 2.5.0
Do we have an example of something that is not working correctly because of locks or lack of locks in our code?
- 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.
Also available in: Atom
PDF