std::deque Thread Saftey Situtation

On 30 Aug., 00:49, NvrBst <nvr...@gmail.c omwrote:

I've read a bit online seeing that two writes are not safe, which I
understand, but would 1 thread push()'ing and 1 thread pop()'ing be
thread-safe? *

No

Basically my situation is the follows:

--Thread 1--
1. Reads TCPIP Buffer
2. Adds Buffer to Queue (q.size() to check queue isn't full, and
q.push_back(... ))
3. Signals Reading Thread Event & Goes Back to Wait for More Messages
on TCPIP

--Thread 2--
1. Waits for Signal
2. while(q.size() 0) myStruct = q.front();
2a. processes myStruct (doesn't modify it but does copy some
information for another queue).
2b. delete[] myStruct.Buffer
2c. q.pop_front();
3. Goes back to waiting for single

I've run the app for a few hours (basically saturating the TCPIP
traffic) and there were no apparent problems. *The "processes
myStruct" takes a while and I can't have the push_back(...) thread
locked while processing is working.

If it did not fail, you were simply lucky. You will surely run out of
luck some day.

>
I can add Critical Section locks around the ".size()", ".front()" and/
or ".pop_front ()/.push_back()", but my first inclination is that this
is thread safe?

It is thread-safe only when you protect everything with a mutex (which
could very well be a windows CRITICAL_SECTIO N).

[snip]

If Critical Sections are needed, would it just be fore the
".pop_front ()/.push_back()"? or all member functions I'm using
(.size()/.front()).

On all of them.

>
Thanks. *Any information would be greatly appreciated. *These are the
only two threads/functions accessing the queue. *I currently
implemented it with locking on all just to be safe, but would like to
remove the locking if it is not needed, or fine tune it.

Did you measure the performance without a lock? Presumably, it would
not really make a difference.

/Peter

Ahh thanks for the clarification. I was sorta expecting it not to be
thead safe -_-.

Did you measure the performance without a lock? Presumably, it would
not really make a difference.

The clean locking way IE

lock();
while(q.size() 0) {
Process((.front ().Buffer);
delete[] etc
..pop_front();
}
unlock();

Was way to slow (because of the "Process(.. )"). Moving the lock
inside the while loop, and adding some tmp vars, and also managing the
"q.size()" in the head of the while loop made the code a little more
ugly, but I believe the performance isn't too much worst; just seemed
to me that if I wanted to increase the performance, tackling the
locking/unlocking that's inside the while loop might be a good thing
to look into (the while loop can run hundreds, maybe thousands of
times depending how fast the other thread is pumping the queue up).

Thanks

On 30 Aug., 01:27, NvrBst <nvr...@gmail.c omwrote:

Ahh thanks for the clarification. *I was sorta expecting it not to be
thead safe -_-.

Did you measure the performance without a lock? Presumably, it would
not really make a difference.

The clean locking way IE

lock();
while(q.size() 0) {
Process((.front ().Buffer);
delete[] etc
.pop_front();}

unlock();

This is not at all clean. You lock while processing many elements,
while you only should lock for the short duration where you pop/push
an element.

Search for "condition variables" for an idea on how to do this kind of
stuff. A producer-consumer structure is really quite basic and most
useful - but it must be implemented correctly.

>
Was way to slow (because of the "Process(.. )"). *Moving the lock
inside the while loop, and adding some tmp vars, and also managing the
"q.size()" in the head of the while loop made the code a little more
ugly, but I believe the performance isn't too much worst; just seemed
to me that if I wanted to increase the performance, tackling the
locking/unlocking that's inside the while loop might be a good thing
to look into (the while loop can run hundreds, maybe thousands of
times depending how fast the other thread is pumping the queue up).

A low level mutex is a very fast beast unless you have heavy
contention. If your data is expensive to copy, that might be a
bottleneck, but wait dealing with that until you have gotten your
basic structure to work.

/Peter

A low level mutex is a very fast beast unless you have heavy

contention. If your data is expensive to copy, that might be a
bottleneck, but wait dealing with that until you have gotten your
basic structure to work.

/Peter

The information is too much to copy. When I said clean, I ment more-
so clean to look at (understand). What I have no looks like this:

Enter();
while(.size() 0) {
Leave();
Process();
Enter();
}
Leave();

It works fine but as you might guess a little harder to understand, I
left out some temp variables, but basically it's not as clean to look
at as:

Enter();
while(.size() 0) Process();
Leave();
Is the top way bad? Is there a consumer-producer model I should be
using instead (basically the TCPIP reading can't be blocked, and
processing the reading queue takes a while; it usally sometimes hits
0, but the "while(.siz e()) {..}" is usally running 75% of the time.

Thanks

On 30 Aug., 01:53, NvrBst <nvr...@gmail.c omwrote:

A low level mutex is a very fast beast unless you have heavy
contention. If your data is expensive to copy, that might be a
bottleneck, but wait dealing with that until you have gotten your
basic structure to work.

/Peter

The information is too much to copy. *When I said clean, I ment more-
so clean to look at (understand). *What I have no looks like this:

Enter();
while(.size() 0) {
Leave();
Process();
Enter();}

Leave();

This is not so easy to understand - even with correct indentation. And
it is not exception-safe.

>
It works fine but as you might guess a little harder to understand, I
left out some temp variables, but basically it's not as clean to look
at as:

Enter();
while(.size() 0) Process();
Leave();

Is the top way bad? *Is there a consumer-producer model I should be
using instead (basically the TCPIP reading can't be blocked, and
processing the reading queue takes a while; it usally sometimes hits
0, but the "while(.siz e()) {..}" is usally running 75% of the time.

As I said, you should look for condition variables. Perhaps boost has
something for you. I am not sure that they have a producer-consumer
class, but they have the building blocks you need, and quite likely
also an example that you can use as a skeleton.
I am confident that there are also other solutions out there.

/Peter

On Aug 29, 7:45 pm, NvrBst <nvr...@gmail.c omwrote:

Everything I've written is pseudo-code, not my actual
code. I can't get the Indentations right here, and would be too hard
to follow if I just paste, but I can outline the basics:

--Pumping Thread--
WaitForReadSign le();
ReadTCPIP(Buffe r, size, ...);
//Copies Buffer, and makes myStruct
m_Q[TCPIP].qLock.Enter();
q.push_back(myS truct);
m_Q[TCPIP].qLock.Leave();
SetEvent(recvie Event);

--Processing Thread--
WaitForObjects( recvieEvent);
m_Q[TCPIP].qLock.Enter();
while(q.size() 0) {
myStruct = q.front();
q.pop_front();
m_Q[TCPIP].qLock.Leave();
ProcessData(myS truct);
m_Q[TCPIP].Enter();}

m_Q[TCPIP].Leave();

Both threads are in loops, and there are stuff I left out for clarity,
but this, more-than-less, is the structure. I do have try{} finally{}
blocks set up (to ensure that for every "Enter()" there is a
"Leave()"). I left them out for clarity. ....

Enough try-catch blocks will work, although I'd find proofreading them
a bit much for my taste. A thin wrapper class would get the
proofreading time per instance down to practically instant.

Please pardon my use of agonizing detail in the following. A really
thin wrapper class WrapMutex should:
* take the actual mutex as a non-const reference parameter in its
constructor
* store a reference to the actual mutex as private data
** I think this implies not default-constructable, as a parameter is
needed to initialize the reference. Usually not a good trait, but
acceptable for this class.
* call Enter on the mutex in its inline constructor
* call Leave on the mutex in its inline non-virtual destructor
* not be copyable
** We should lose the default assignment operator because of the
reference data member, but the copy constructor needs to be rendered
unusable. Besides the textbook method, deriving from
boost::noncopya ble will work.

Then your code

m_Q[TCPIP].qLock.Enter();
while(q.size() 0) {
myStruct = q.front();
q.pop_front();
m_Q[TCPIP].qLock.Leave();
ProcessData(myS truct);
m_Q[TCPIP].qLock.Enter();
}
m_Q[TCPIP].qLock.Leave();

can be replaced by

some_struct_typ e myStruct;
while(q.size() 0) {
{
WrapMutex(m_Q[TCPIP].qLock);
myStruct = q.front();
q.pop_front();
}
ProcessData(myS truct);
}

As I said, you should look for condition variables. Perhaps boost has
something for you. I am not sure that they have a producer-consumer
class, but they have the building blocks you need, and quite likely
also an example that you can use as a skeleton.
I am confident that there are also other solutions out there.

I'm unsure what boost is,

The main website is http://www.boost.org/ ; it's a very extensive C++
library with a noticeable learning curve. I personally am nonfluent
in it. (I currently use mainly the program-correctness, conditional
compilation of templates, and Boost.Preproces sor sub-libraries.)

On Aug 29, 5:49 pm, NvrBst <nvr...@gmail.c omwrote:

I've read a bit online seeing that two writes are not safe, which I
understand, but would 1 thread push()'ing and 1 thread pop()'ing be
thread-safe?

As both push() and pop() are non-const operations, no. (In general,
non-const operations are "writing" to the data structure.)

Assuming the data type has no mutable data members: Even one non-const
operation in parallel with any number of const operations on the same
data structure is technically unsafe. Also, any operation that
invalidates iterators is unsafe when another thread has iterators in
use (for a std::deque, inserting elements at all comes to mind).

There are plausibly some more subtle issues that an expert in the
field would notice.

Ahh thank you. I find examples a lot easier to follow, one question:

some_struct_typ e myStruct;
while(q.size() 0) {
* {
* WrapMutex(m_Q[TCPIP].qLock);
* myStruct = q.front();
* q.pop_front();
* }
* ProcessData(myS truct);

}

This would imply the ".size()" doesn't have to be wrapped in the
CriticalSection s? The reasion I put the locks/unlocks outside the
whileloop was solely for that reasion. I actually already have a
wraper like you listed "CriticalSectio n::Owner
sLock(m_Q[TCPIP].qLock);" which does what you outlined, but I failed
to realize I could put empty curly brackets around the section I want,
to force it out of scope :)

Most my confusion comes from my C# background (java, and then C# is
what I grew up with), and in C#'s Queue, the size property is stored
as an Int32 which, when reading, is an atomic operation on mostly all
processors. Meaning it even read it before another thread changes it,
or after; either case is fine (thread safe) if there are only two
threads, and each thread either pop's or push's, thus can't cause an
exception. This is because in C# the size doesn't get incremented
untill after the insertion is completed, so exceptions can't occure in
my situation.

I was thinking ".front()" should also be thread safe for the same (C#-
Based) reasion, since it's a 32bit applications, and .front() should
be returning a pointer to the first element. Since the thread
calling .front() is the only thread who's removing elements, and since
this thread knows that ".size()" shows one element, then I would of
assumed ".front()" would be thread-safe as well.

Since .size() was a method call instead of a property, I was thinking
there might be more to it in c++... Which I'm not confused on again:
".size()" ".front()" would both be thread-safe operations for a thread
thats removes elements (being the only thread that removes items)? In
C# Enqueue/Dequeue arn't thread safe, but again, depends on
implementations , which is why I asked here; I would be suprised if
they were, but thought maybe.

So in essance I can change it to the following if I wanted to fine
tune the thread-safty locking to it's bare minimium in my example (and
remove my try statment to boot, yay)?

some_struct_typ e myStruct;
while(q.size() 0) {
myStruct = q.front();
{
WrapMutex(m_Q[TCPIP].qLock);
q.pop_front();
}
ProcessData(myS truct); }
Thanks, NB

"Which I'm not confused on again" -"Which I'm now confused on
again". Sorry typo's, Amung others; this one change my meaning a bit
more than I wanted.