 |
C++Talk.NET
C++ language newsgroups
|
| View previous topic :: View next topic |
| Author |
Message |
sagenaut@yahoo.com
Guest
|
 Posted: Sat Jun 24, 2006 2:38 pm Post subject: thread safe |
 |
|
I have a monitor like class and a thread-safe queue:
class Monitor {
public:
void lock();
void unlock();
wait();
signal();
...
};
class MTQueue {
public:
void put();
void get();
...
private:
std::queue<item> queue_
Monitor getLock_;
Monitor putLock_;
Monitor emptyLock_;
Monitor serializeLock_;
...
};
Here is the get() and put() implementation:
void put()
{
putLock_.lock();
queue_.push(item);
emptyLock_.signal();
putLock_.unlock();
}
void get()
{
getLock_.lock();
while (queue_.empty()) emptyLock_.wait();
item = queue_.front();
queue_.pop();
getLock_.unlock();
}
Are the put() and get() thread safe? I used two locks for
serialization and one lock for signaling. This allows two threads
access to the queue: one for put and the other for get. Normal
implementation would use only one lock and allows only one thread to
access queue:
void put()
{
serializeLock_.lock();
queue_.push(item);
serializeLock_.signal();
serializeLock_.unlock();
}
void get()
{
serializeLock_.lock();
while (queue_.empty()) serializeLock_.wait();
item = queue_.front();
queue_.pop();
serializeLock_.unlock();
}
Which is the right way to implemt the put() and get()? Thanks in
advance.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
 |
Heinz Ozwirk
Guest
|
| Posted: Sun Jun 25, 2006 6:45 pm Post subject: Re: thread safe |
|
|
"sagenaut (AT) yahoo (DOT) com" <sagenaut (AT) gmail (DOT) com> schrieb im Newsbeitrag
news:1151088769.030803.190970 (AT) b68g2000cwa (DOT) googlegroups.com...
| Quote: | I have a monitor like class and a thread-safe queue:
class Monitor {
public:
void lock();
void unlock();
wait();
signal();
...
};
class MTQueue {
public:
void put();
void get();
...
private:
std::queue<item> queue_
Monitor getLock_;
Monitor putLock_;
Monitor emptyLock_;
Monitor serializeLock_;
...
};
Here is the get() and put() implementation:
void put()
{
putLock_.lock();
queue_.push(item);
emptyLock_.signal();
putLock_.unlock();
}
void get()
{
getLock_.lock();
while (queue_.empty()) emptyLock_.wait();
item = queue_.front();
queue_.pop();
getLock_.unlock();
}
Are the put() and get() thread safe? I used two locks for
serialization and one lock for signaling. This allows two threads
access to the queue: one for put and the other for get.
|
Since there are no threads or locks in standard C++ (at least not yet) you
should better ask in a group for your system. So without knowing what
exactly your Monitor class really does, the following comments may be of
little value.
No. I don't think your code is thread safe. You should better use a single
lock for reading and writing. Inserting a new item to a container might use
iterators or pointers and while these are used, they must not become
invalid. OTH, removing an item from a container may invalidate all
iterators/pointers to elements in that container might comflict with
simultanous insertion. It is the container you must protect, not a single
operation.
Also, you should unlock the container as early as possible. Therefor it
would be better if put would unlock the container before signaling. But even
more important, get must not keep the container locked while it is waiting
for something to be written to it. How can another thread write something
into the queue while it is locked?
aka "working"
| Quote: | implementation would use only one lock and allows only one thread to
access queue:
void put()
{
serializeLock_.lock();
queue_.push(item);
serializeLock_.signal();
serializeLock_.unlock();
}
void get()
{
serializeLock_.lock();
while (queue_.empty()) serializeLock_.wait();
item = queue_.front();
queue_.pop();
serializeLock_.unlock();
}
Which is the right way to implemt the put() and get()? Thanks in
advance.
|
IMHO none of above. Protecting data against concurrent manipulation and
sending messages from one thread to another are different tasks, so there
should be different types objects or at least different instances of them.
Some systems use mutexes to protect critical sections of code and events to
signal an important change of an object's state. And there may be even more
types of syncronisation objects. If your Monitor can do both it might be OK,
but in general it seems better to use different classes for different tasks.
HTH
Heinz
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
Thomas Richter
Guest
|
| Posted: Sun Jun 25, 2006 6:48 pm Post subject: Re: thread safe |
|
|
sagenaut (AT) yahoo (DOT) com <sagenaut (AT) gmail (DOT) com> wrote:
| Quote: | I have a monitor like class and a thread-safe queue:
|
First of all, C++ doesn't specify threats and mutexes and
the like, and thus your question goes possibly to
the wrong forum. How mutuxes perform is up to the underlying
operating system providing such services. Let's assume POSIX
for the time being.
However:
| Quote: | void put()
{
putLock_.lock();
queue_.push(item);
emptyLock_.signal();
putLock_.unlock();
}
void get()
{
getLock_.lock();
while (queue_.empty()) emptyLock_.wait();
item = queue_.front();
queue_.pop();
getLock_.unlock();
}
Are the put() and get() thread safe?
|
No, unless the queue has some synchronization handling of its own.
A thread running into get() might preempt any time by a thread running into
put, thus the state seen within get() need not to be consistent. There should
be exactly one access mutex to the thread that is used by both using get()
and put(). However, you *cannot* hold that mutex within the wait loop of
get() as this will cause the getter to block the access, and thus prevents
any thread to run into put(). Thus, this would be essentially a deadlock.
You should first wait for the emptyLock, then try to get access to the
queue, then try to remove an element. Concerning emptyLock: This *should not*
be a mutex at least for the operating systems I know. Locking a mutex within
one thread and releasing it in another is at least undefined within POSIX
(and of course undefined within C++ which has no say about threads and
mutexes in first place), so a semaphore would be the way to go.
| Quote: | void put()
{
serializeLock_.lock();
queue_.push(item);
serializeLock_.signal();
serializeLock_.unlock();
}
void get()
{
serializeLock_.lock();
while (queue_.empty()) serializeLock_.wait();
item = queue_.front();
queue_.pop();
serializeLock_.unlock();
}
|
That's broken, too. See above.
| Quote: | Which is the right way to implemt the put() and get()? Thanks in
advance.
|
None of the above. (-: See the comments.
So long,
Thomas
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in to check your private messages
Log in
