concurrent_hash_map::erase deadlock when using erase(key) while holding accessor on same thread

[gnobal]

Hi,

I just wanted to point out some behaviour I ran into (by mistake) that isn't documented. Consider the following code:

typedef concurrent_hash_map MapT;

MapT m;

m.insert(make_pair(1,0));

MapT::accessor a;

m.find(a, 1);

// Assuming 1 was found

m.erase(1);

In my experience the last line (call to erase) deadlocks. I guess this is because I'm doing the erase() on the same thread as the find() with the accessor still locking the entry, but in the documentation I believe it doesn't say anything about this case and that erasing like this should work. For me the solution was to erase using the accessor, as follows:

m.erase(a);

Hope this helps someone. I'm using the first release of TBB 4.0, but I got the same behaviour with 3.0 update 8.

[Anton_M_Intel]

Thank you! We will improve the documentation. We had plans to fix it in the container but it seems deferred for indefinite time (until some customer comes to us and tell to fix... :) ).

[jimdempseyatthecove]

In your mind then, just what do you suggest accessor "a" references after the erase (assuming erase permitted to continue while a/any referenceheld)?

When two threads are involved, the ereasing thread should block untilall references end.

I suppose it is possible to write the dtor of the accessor to examine an erase flag (and back referenc to container), however I do not believe this is the proper way to write an accessor.

Jim Dempsey

[gnobal]

The short answer to your question is that I don't care what "a" references after the erase.

And here's the long answer:

I haven't really considered the question you're posing. My point of view is one of a library user, not an implementer's. I just read the following from the documentation of bool erase(const Key& key):

Searches table for pair with given key. Removes the matching pair if it exists. If thereis an accessor pointing to the pair, the pair is nonetheless removed from the table butits destruction is deferred until all accessors stop pointing to it.

and the following from the documentation of bool erase(accessor& item_accessor):

Removes pair referenced by item_accessor. Concurrent insertion of the same keycreates a new pair in the table.Removes pair referenced by item_accessor. Concurrent insertion of the same key creates a new pair in the table.

and concluded that I will be able to erase using the key at any given point in the code. My original observation was that nothing is said regarding the use of the overload erase(const Key&) from the same thread as the thread that executed a find() and is still keeping an accessor to that item (the result of which is a deadlocked thread.) Using erase(accessor&) works well and is on par with the documentation, which is good for me.

Like you said, the concurrent_hash_map indeed keeps the item until all references end, but it doesn't block erasures. It just marks the item as erased until the last reference to it is gone. This is also in the documentation:

CAUTION: Though there can be at most one occurrence of a given key in the map, there may beother key-value pairs in flight with the same key. These arise from the semantics of theinsert and erase methods. The insert methods can create and destroy a temporarykey-value pair that is not inserted into a map. The erase methods remove a key-valuepair from the map before destroying it, thus permitting another thread to construct asimilar key before the old one is destroyed.

Amit