tags:

views:

568

answers:

3

Hi,

I have a std::map that is used by multiple threads to store data. The map is declared like this:

std::map<int, Call> calls;

From each thread, I have to acquire a mutex lock, obtain a pointer or reference to the object belonging to that thread, then release the mutex lock. I can modify the object after that because each object is used only by one thread. As soon as the thread dies, the corresponding pair in the map would also get deleted.

I would like to know the best way to implement this. I was thinking of two ways:

1) I know this one could look outrageously crazy, but still

std::map<int, Call> calls;
...

{
    mutex.lock();
    Call* callptr = &calls[id];
    mutex.unlock();

   // use callptr
}

or 2) I think this one looks more sensible

std::map<int, std::auto_ptr<Call> > calls;

...

{
    mutex.lock();
    std::auto_ptr<Call> callptr = map[id];
    mutex.unlock();

    // use callptr

    mutex.lock();
    map[id] = callptr;
    mutex.unlock();
}

The threads actually are created in a different dll and I don't have the code for that. This dll that I'm writing now gets imported by that dll and used. So it has to be implemented with std::map only, but could anyone tell me if one of these methods is ok or if there are ways to make it more stable.

Thanks

+4  A: 

You should use iterators:

mutex.lock();

std::map<int, Call>::iterator callptr = calls.find(id);
callptr->second.foo();
...

mutex.unlock();

Your first solution with pointers is problematic, because the lifetime of the object in the map is uncertain - it may be moved when the tree is rebalanced when elements are inserted or deleted.

Your second solution won't work at all, because std::auto_ptr does not fulfil the requirements for mapped_type of std::map - mostly because its copy constructor and operator= don't actually copy. You likely won't get a compiler error, but you'll get very weird behavior at run-time.

Pavel Minaev
Thanks for the iterator suggestion, Pavel. And I didn't know that about smart pointers and containers.
Sahasranaman MS
Sorry, you're wrong. The lifetime of an object in a std::map begins when it's inserted and ends when it's removed or the map deleted. Rebalancing changes internal node pointers, but touches neither Key nor Value
MSalters
+2  A: 

Don't use auto_ptr's in STL containers. DON'T. The implementors are actually required to try and make its use there a compilation error. Standard containers tend to copy things around. That's a very wrong thing with auto_ptr.

EFraim
+3  A: 

According to me Thread Local storage is the best option for you.

If you need thread specific data, you can make use of Thread Local Storage and completely eliminate the need for map and mutex locking.

Canopus
Yes I know, but unfortunately I can't do that because threads get created in a different DLL for which I don't have the code!
Sahasranaman MS
That is not a problem. You need to call TlsAlloc once per process, not once per thread.
MSalters