std::map multithreaded access, lock needed?

[Christopher Pisz]

With the recent acceptance of multithreading questions, I assume this
question is on topic. If not, let me know.

I have a singleton is going to be accessed by any threads. My singleton
class contains a static std::map that will be read and written to. I am
wondering if I need to lock it when being read from and written to? The sgi
docs lead me to beleive I do... See LoggerWindow::OpenLog below.

// in the .h

#include "LogWindow.h"

#include <map>

[snip]

class LoggerWindow
{
public:

/**
* Constructor
*/
LoggerWindow();

/**
* Destructor
*/
~LoggerWindow();

/**
* Creates child windows for one log
*/
void OpenLog(std::string title);

private:

[snip]

static HANDLE m_frameCreated; // Event that is signalled when the frame
window has been created

/** Pointers to the LogWindow instances that are the MDI children */
typedef std::map<std::string, LogWindow *> LogWindows;
static LogWindows m_logWindows;

[snip]
};

// in the .cpp

#include "LoggerWindow.h"

[snip]

//----------------------------------------------------------------------------------------------------------------------
// Initialize static members of the LoggerWindow class

[snip]

HANDLE LoggerWindow::m_frameCreated = CreateEvent(NULL, TRUE, FALSE, NULL);
LoggerWindow::LogWindows LoggerWindow::m_logWindows;

[snip]

[snip]

//----------------------------------------------------------------------------------------------------------------------
void LoggerWindow::OpenLog(std::string title)
{
// This operation cannot execute until the frame window is fully created
WaitForSingleObject(m_frameCreated, INFINITE);

// Is a lock needed here????

// Check if the LogWindow already exists
LogWindows::iterator it = m_logWindows.find(title);
if( it == m_logWindows.end() )
{
// Create a Log Window which is an MDI child of this window
LogWindow * logWindow = new LogWindow(m_hwndClient, title);
m_logWindows[title] = logWindow;
}

// Is an unlock needed here????
}

[snip]

 

[James Kanze]

With the recent acceptance of multithreading questions, I assume this
question is on topic. If not, let me know.

I have a singleton is going to be accessed by any threads. My singleton
class contains a static std::map that will be read and written to. I am
wondering if I need to lock it when being read from and written to? The sgi
docs lead me to beleive I do... See LoggerWindow::OpenLog below.

// in the .h

#include "LogWindow.h"

#include <map>

[snip]

class LoggerWindow
{
public:

/**
* Constructor
*/
LoggerWindow();

/**
* Destructor
*/
~LoggerWindow();

/**
* Creates child windows for one log
*/
void OpenLog(std::string title);

private:

[snip]

static HANDLE m_frameCreated; // Event that is signalled when the frame
window has been created

/** Pointers to the LogWindow instances that are the MDI children */
typedef std::map<std::string, LogWindow *> LogWindows;
static LogWindows m_logWindows;

[snip]

};

// in the .cpp

#include "LoggerWindow.h"

[snip]

//----------------------------------------------------------------------------------------------------------------------
// Initialize static members of the LoggerWindow class

[snip]

HANDLE LoggerWindow::m_frameCreated = CreateEvent(NULL, TRUE, FALSE, NULL);
LoggerWindow::LogWindows LoggerWindow::m_logWindows;

[snip]

[snip]

//----------------------------------------------------------------------------------------------------------------------
void LoggerWindow::OpenLog(std::string title)
{
// This operation cannot execute until the frame window is fully created
WaitForSingleObject(m_frameCreated, INFINITE);

// Is a lock needed here????

// Check if the LogWindow already exists
LogWindows::iterator it = m_logWindows.find(title);
if( it == m_logWindows.end() )
{
// Create a Log Window which is an MDI child of this window
LogWindow * logWindow = new LogWindow(m_hwndClient, title);
m_logWindows[title] = logWindow;
}

// Is an unlock needed here????

}

[snip]

 

[James Kanze]

[Sorry for that last posting. Google seems to have screwed
up.]

I have a singleton is going to be accessed by any threads. My
singleton class contains a static std::map that will be read
and written to. I am wondering if I need to lock it when being
read from and written to? The sgi docs lead me to beleive I
do...

Formally, it depends on the guarantees given by your
implementation, but generally, all of the implementations give
the SGI guarantee for std::map (and I'm pretty sure this is what
will be adopted in the next version of the standard). If any
thread may modify the object, all accesses, from all threads,
must be synchronized in some way.

See LoggerWindow::OpenLog below.

// in the .h

#include "LogWindow.h"

#include <map>

[snip]

class LoggerWindow
{
public:

/**
* Constructor
*/

Just a nit, but that's really an example of a useless comment.
The sort of thing that you want to avoid. (On the other hand,
it's often useful to document what state the constructor leaves
the object in.)

LoggerWindow();

/**
* Destructor
*/
~LoggerWindow();

/**
* Creates child windows for one log
*/
void OpenLog(std::string title);

private:

[snip]

static HANDLE m_frameCreated; // Event that is signalled when the frame
window has been created

/** Pointers to the LogWindow instances that are the MDI children */
typedef std::map<std::string, LogWindow *> LogWindows;
static LogWindows m_logWindows;

[snip]

};

// in the .cpp

#include "LoggerWindow.h"

[snip]

//----------------------------------------------------------------------------------------------------------------------
// Initialize static members of the LoggerWindow class

[snip]

HANDLE LoggerWindow::m_frameCreated = CreateEvent(NULL, TRUE, FALSE, NULL);
LoggerWindow::LogWindows LoggerWindow::m_logWindows;

The above isn't really what is usually understood by
"singleton". It's just an everyday static variable.

[snip]

[snip]

//----------------------------------------------------------------------------------------------------------------------
void LoggerWindow::OpenLog(std::string title)
{
// This operation cannot execute until the frame window is fully created
WaitForSingleObject(m_frameCreated, INFINITE);

// Is a lock needed here????

// Check if the LogWindow already exists
LogWindows::iterator it = m_logWindows.find(title);
if( it == m_logWindows.end() )
{
// Create a Log Window which is an MDI child of this window
LogWindow * logWindow = new LogWindow(m_hwndClient, title);
m_logWindows[title] = logWindow;
}
// Is an unlock needed here????
}

[snip]

If the OpenLog function can be called from different threads,
then you definitely need a lock. (I'd use some sort of scoped
lock.) How could it possibly be otherwise?

Another alternative, of course, would be to organize your code
so that OpenLog is only called from a single thread. (From the
names, it looks like you're dealing with GUI code---it's
generally best if all of the GUI management takes place in a
dedicated thread---other threads would just post an event
requesting the creation of a log window.)

 

[CodeCracker]

Yes you will need to put the lock before handling the map. Since other
thread might have created the entry in the map and would might be
deleting it once they are done. Ifit is created only once and is never
deleted during operation then you would not need the locking
mechanism.

Thanks,

 

[James Kanze]

Yes you will need to put the lock before handling the map.
Since other thread might have created the entry in the map and
would might be deleting it once they are done. Ifit is created
only once and is never deleted during operation then you would
not need the locking mechanism.

That doesn't sound to clear to me. Even if you never delete
anything, you need to synchronize access. The only case where
you wouldn't need to synchronize access is if the map were
completely filled before you started threading (or before it
became visible to other threads); as soon as any thread modifies
anything, all accesses must be synchronized.

 

[Andre Kostur]

Really? I missed where multithreading became on-topic for comp.lang.c++...

You have to check your library's documentation as to what thread-safety
guarantees it has... but the one that seems to be the most common (among
library implementations that are intended to be run in a MT environment):

- Different instances of a container are independant (no locks required)
- Multiple readers into a single container is safe (no locks required)
- Writing into a single container requires protection to prevent multiple
writers, and potentially other readers (It may not be safe to attempt to
read & write into the container at the same time. Offhand I can think that
attempting to do a find into a map at the same time as an insertion is
going on might catch the internal implementation of the map in an
inconsistent state.)

 

[James Kanze]

innews:aa629021-ef92-4c4b-a498-102877a50a9a@k39g2000hsf.googlegroups.com:

Really? I missed where multithreading became on-topic for
comp.lang.c++...

When was it ever off-topic? The topic here is the C++ language.
Multithreading is definitely relevant to the C++ language, and
has been since C++ implementations which support multi-threading
have become widespread. (Obviously, detailed discussion of a
particular threading model is better handled elsewhere, and
detailed discussion of a particular, non-portable API isn't on
topic. But neither were the question here.)

You have to check your library's documentation as to what
thread-safety guarantees it has... but the one that seems to
be the most common (among library implementations that are
intended to be run in a MT environment):

- Different instances of a container are independant (no locks required)
- Multiple readers into a single container is safe (no locks required)
- Writing into a single container requires protection to prevent multiple
writers, and potentially other readers (It may not be safe to attempt to
read & write into the container at the same time. Offhand I can think that
attempting to do a find into a map at the same time as an insertion is
going on might catch the internal implementation of the map in an
inconsistent state.)

Not "It may not be safe", but "It is undefined behavior" (to
attempt to read and write into the container at the same time).
And it isn't only related to what the code might or might not
do; there are memory synchronization issues involved as well.

 

[Chris Thomasson]

message
(e-mail address removed)...

That doesn't sound to clear to me. Even if you never delete
anything, you need to synchronize access. The only case where
you wouldn't need to synchronize access is if the map were
completely filled before you started threading (or before it
became visible to other threads); as soon as any thread modifies
anything, all accesses must be synchronized.

Humm... Well, if you use std::map, then you are 100% correct... However, I
feel the need to point out that one can create their own map structure which
does not need to synchronize reads in the sense that PDR algorithms can be
employed to manage the memory visibility concerns wrt removing items and
deallocating then from the collection. Think RCU...

 

[James Kanze]

Humm... Well, if you use std::map, then you are 100%
correct... However, I feel the need to point out that one can
create their own map structure which does not need to
synchronize reads in the sense that PDR algorithms can be
employed to manage the memory visibility concerns wrt removing
items and deallocating then from the collection. Think RCU...

One can imagine just about any contract one wishes. But the
interface of std::map pretty much makes anything stronger than
the SGI guarantees useless anyway. Anytime you return a means
of access (e.g. reference, pointer or iterator) into the data of
the map, you need to synchronize somehow at a higher level, in
order to prevent the referenced object from disappearing under
your feet, or otherwise being modified without your knowledge.
You need some sort of synchronization for the larger context
anyway.

 

[Christopher Pisz]

Humm... Well, if you use std::map, then you are 100% correct... However, I
feel the need to point out that one can create their own map structure
which does not need to synchronize reads in the sense that PDR algorithms
can be employed to manage the memory visibility concerns wrt removing
items and deallocating then from the collection. Think RCU...

I do not understand this at all. I tryed googling PRD algorithms and came up
with nothing. I found that RCU means "Read Copy Update" and is supposed to
be a replacement for read and write locking. However, I am unable to find
any examples on using the algorithm as it seems to be specific to a
processor and possibly even asm calls? Anyway, I am very new to multi
threading in general and could use a few resources that would help make a
lock free map. If you know of any please throw them my way.

 

[CodeCracker]

That doesn't sound to clear to me.  Even if you never delete
anything, you need to synchronize access.  The only case where
you wouldn't need to synchronize access is if the map were
completely filled before you started threading (or before it
became visible to other threads); as soon as any thread modifies
anything, all accesses must be synchronized.

What was not clear? You said " as soon .. modifies ...must be
synchronized"
I said the same thing. The lock would be used to synchronize the
content since only one will be permitted to reach it at a time.