Project

General

Profile

Bug #6322

Ensure lock is obtained prior to calls to Hazelcast IMap.put()

Added by Chris Jones over 5 years ago. Updated over 3 years ago.

Status:
Rejected
Priority:
Normal
Assignee:
Category:
metacat
Target version:
Start date:
12/20/2013
Due date:
% Done:

0%

Estimated time:
Bugzilla-Id:

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.

History

#1 Updated by ben leinfelder over 5 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...

#2 Updated by Matt Jones over 5 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.

#3 Updated by ben leinfelder over 5 years ago

  • Target version changed from 2.4.0 to 2.5.0

#4 Updated by ben leinfelder over 3 years ago

Do we have an example of something that is not working correctly because of locks or lack of locks in our code?

#5 Updated by Chris Jones over 3 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.

Also available in: Atom PDF