List<T> thread safety question

R

Rainer Queck

Hi NG,

one more question about thread safety of generic lists.
Let's assume a generic list:
List<MyTyp> aList = new List<MyType>();

Would it be a problem if one thread removes elements from the list like
MyTyp x = aList[0];
aList.Remove(x);

While in the same momen a other thread adds a elemen like
MyType y = new MyType();
aList.Add(y);

Is it safe to assume that these two operations in combination are OK?

Thanks for hints and help?
Rainer
 
N

Nick Hounsome

Rainer Queck said:
Hi NG,

one more question about thread safety of generic lists.
Let's assume a generic list:
List<MyTyp> aList = new List<MyType>();

Would it be a problem if one thread removes elements from the list like
MyTyp x = aList[0];
aList.Remove(x);

While in the same momen a other thread adds a elemen like
MyType y = new MyType();
aList.Add(y);

Is it safe to assume that these two operations in combination are OK?

No - it will definitely fail.

This is a classic threading problem and cobbling together a threadsafe Q<T>
is a good exercise that you should really work out for yourself.

In outline you need to protect the q whilst accessing it; have a mechanism
to block if removing from an empty q;have a mechanism to unblock other
threads when adding to a q and possibly more if you want a maximum q size.
 
R

Rainer Queck

Hi Nick,

thanks for responding!
one more question about thread safety of generic lists.
Let's assume a generic list:
List<MyTyp> aList = new List<MyType>();

Would it be a problem if one thread removes elements from the list like
MyTyp x = aList[0];
aList.Remove(x);

While in the same momen a other thread adds a elemen like
MyType y = new MyType();
aList.Add(y);

Is it safe to assume that these two operations in combination are OK?

No - it will definitely fail.
This is what I expected, just wanted to make sure.

Would this be thread safe?:

List<MyTyp> aList = new List<MyType>();

// one thread removes elements from the list like
Monitor.Enter(aList)
try
{
MyTyp x = aList[0];
aList.Remove(x);
}
finally{
Monitor.Exit(aList)
}

// While in the same momen a other thread adds a element like

MyType y = new MyType();
Monitor.Enter(aList)
try
{
aList.Add(y);
}
finally{
Monitor.Exit(aList);
}

Regards
Rainer
 
N

Nick Hounsome

Rainer Queck said:
Hi Nick,

thanks for responding!
one more question about thread safety of generic lists.
Let's assume a generic list:
List<MyTyp> aList = new List<MyType>();

Would it be a problem if one thread removes elements from the list like
MyTyp x = aList[0];
aList.Remove(x);

While in the same momen a other thread adds a elemen like
MyType y = new MyType();
aList.Add(y);

Is it safe to assume that these two operations in combination are OK?

No - it will definitely fail.
This is what I expected, just wanted to make sure.

Would this be thread safe?:

List<MyTyp> aList = new List<MyType>();

// one thread removes elements from the list like
Monitor.Enter(aList)
try
{
MyTyp x = aList[0];
aList.Remove(x);
}
finally{
Monitor.Exit(aList)
}

// While in the same momen a other thread adds a element like

MyType y = new MyType();
Monitor.Enter(aList)
try
{
aList.Add(y);
}
finally{
Monitor.Exit(aList);
}

Regards
Rainer

firstly"lock" is neater than Monitor (it uses Monitor behind the scenes)
lock(aList)
{
// do stuff
}

secondly you're solution is OK as far as it goes but it avoids the main
issue which is to do with the question "what if the list is empty when I try
to take something off it?"

1) You cannot test for empty outside of the locked region in wgich you get
the element
2) You must either return something indicating that it was empty or wait
until it isn't empty which will require the Wait and Pulse methods on the
Monitor
3) If you have a fixed length list queue then the same issues apply to the
add method
 
N

Nick Hounsome

Nick Hounsome said:
Rainer Queck said:
Hi Nick,

thanks for responding!
one more question about thread safety of generic lists.
Let's assume a generic list:
List<MyTyp> aList = new List<MyType>();

Would it be a problem if one thread removes elements from the list like
MyTyp x = aList[0];
aList.Remove(x);

While in the same momen a other thread adds a elemen like
MyType y = new MyType();
aList.Add(y);

Is it safe to assume that these two operations in combination are OK?

No - it will definitely fail.
This is what I expected, just wanted to make sure.

Would this be thread safe?:

List<MyTyp> aList = new List<MyType>();

// one thread removes elements from the list like
Monitor.Enter(aList)
try
{
MyTyp x = aList[0];
aList.Remove(x);
}
finally{
Monitor.Exit(aList)
}

// While in the same momen a other thread adds a element like

MyType y = new MyType();
Monitor.Enter(aList)
try
{
aList.Add(y);
}
finally{
Monitor.Exit(aList);
}

Regards
Rainer

firstly"lock" is neater than Monitor (it uses Monitor behind the scenes)
lock(aList)
{
// do stuff
}

secondly you're solution is OK as far as it goes but it avoids the main
issue which is to do with the question "what if the list is empty when I
try to take something off it?"

1) You cannot test for empty outside of the locked region in wgich you get
the element
2) You must either return something indicating that it was empty or wait
until it isn't empty which will require the Wait and Pulse methods on the
Monitor
3) If you have a fixed length list queue then the same issues apply to the
add method

I should add that Pulse will work in certain circumstances PuleseAll is
required in the general case of multiple threads removing stuff from the
list
 
R

Rainer Queck

Hi Nick
firstly"lock" is neater than Monitor (it uses Monitor behind the scenes)
lock(aList)
{
// do stuff
}
I (beginner as I am..) thought, this would work like a critical section,
whereas two threads would only be able to pass through the code one after
the other.
In my case I want to assure a safe Queue access.

So if I interpret your hint, lock actually really locks the object right?
In this case I will rewrite my code from Monitor -> lock Thanks for this
hint!

secondly you're solution is OK as far as it goes but it avoids the main
issue which is to do with the question "what if the list is empty when I
try to take something off it?"
Acutally my real code looks like:

Monitor.Enter(ReceiveQueue);
while(ReceiveQueue.Count>0)
{
Telegram aTlg = ReceiveQueue[0];
ReceiveQueue.Remove(aTlg);
.......

I assume this is OK, or?

Regards
Rainer
 
M

Marc Gravell

I'm not 100% clear on your usage, but three bits of feedback;

1: If (as it appears) you are trying to remove the first item in the
List<T>, you could call RemoveAt(0)

2: under standard "Enter -> Do something -> Exit" usage, it is good practice
to "try" immediately after entering the lock, and exiting the lock in the
finally. This is actually what the lock keyword does for you.

So I would rewrite your posted code as:

lock(ReceiveQueue) {
ReceiveQueue.RemoveAt(0);
}

Much simpler (and robust).

3: if you are implementing a queue in the standard sense, why not use
Queue<T>? This has the normal Enqueue, Dequeue and Peek functions - it might
be a better starting point than List<T> for your usage.

Marc
 
R

Rainer Queck

Hi Marc,
1: If (as it appears) you are trying to remove the first item in the
List<T>, you could call RemoveAt(0)
Actually I fetch the first item from the List, to process it,

Telegram aTlg = ReceiveQueue[0];
ReceiveQueue.Remove(aTlg);
--> now the "aTlg" is processed....

Bringing up this opened my eyes on a other issue I have to take care of but
this is off subject here ;-)
(I am blocking my RecieveQueue untill all messages in it are processed.....)

2: under standard "Enter -> Do something -> Exit" usage, it is good
practice to "try" immediately after entering the lock, and exiting the
lock in the finally. This is actually what the lock keyword does for you.
Good that you brought up this point!
Acually I have done it every where else this way, just not on that peace of
code I copied to the NG.
I will take care of that right next!
So I would rewrite your posted code as:

lock(ReceiveQueue) {
ReceiveQueue.RemoveAt(0);
}

Much simpler (and robust).

3: if you are implementing a queue in the standard sense, why not use
Queue<T>? This has the normal Enqueue, Dequeue and Peek functions - it
might be a better starting point than List<T> for your usage.
Actually I am working with three Lists :
- SendQueue
- ReceiveQueue
- RepeatQueue

With the RepeatQueue, I have to iterate over my whole List to search for
"Telegrams" which are some where in it and then again in some of these cases
I have to remove a "Telegram" which lays somewhere in the middle of the
list.
But you are right, for the Send- and ReceiveQueue the Queue<T> might be the
better choice.

Thans four your response!
Rainer
 
N

Nick Hounsome

Rainer Queck said:
Hi Marc,
1: If (as it appears) you are trying to remove the first item in the
List<T>, you could call RemoveAt(0)
Actually I fetch the first item from the List, to process it,

Telegram aTlg = ReceiveQueue[0];
ReceiveQueue.Remove(aTlg);
--> now the "aTlg" is processed....

Bringing up this opened my eyes on a other issue I have to take care of
but this is off subject here ;-)
(I am blocking my RecieveQueue untill all messages in it are
processed.....)

No it isn't off subject it is what i was getting at with Wait and Pulse.

Your code should look like:

for(;;)
{
Telegram tg = ReceiveQueue.Dequeue(); // blocks thread if empty
tg.Process(); // queue is not blocked here
}

To implement the blocking behaviour you need Wait()

Somewhere else you write:

ReceiveQueue.Enqueue(new Telegram()); // releases Dequeue() blocked on
empty q

All the locking, Pulsing and waiting should be done in yor class in your
Enqueue and dequeue methods
 
M

Marc Gravell

Actually I fetch the first item from the List, to process it,
Telegram aTlg = ReceiveQueue[0];
ReceiveQueue.Remove(aTlg);
--> now the "aTlg" is processed....

Well, even if you want to get the value, I would still *guess* (not proven)
that it would be quicker (nominally) to use RemoveAt - i.e.

Telegram aTlg = ReceiveQueue[0];
ReceiveQueue.RemoteAt(0);
// blah

I haven't profiled it, but that would be my guess.
I am blocking my RecieveQueue untill all messages in it are processed.....

You could try:

while(true) {
lock(ReceiveQueue) {
if(ReceiveQueue.Count==0)
break;
Telegram aTlg = ReceiveQueue[0];
ReceiveQueue.RemoteAt(0);
}
// pulse here perhaps?
}
}

This will still loop until the queue is empty, but will lock/unlock on every
iteration, exiting when empty. It might also need a PulseAll or something at
the end of each iteration to invite the other threads to the party...

Marc
 
J

Jon Skeet [C# MVP]

Rainer said:
one more question about thread safety of generic lists.

<snip>

Rather than respond in detail to this and the other question you've
posted, I suggest you read up on threading in general. That way
hopefully you'll be able to come up with answers without us needing to
find out exactly what your situation is, etc.

I've got a threading tutorial at
http://www.pobox.com/~skeet/csharp/threads

Hopefully that'll provide a good starting point for you.

Jon
 
R

Rainer Queck

Hi Jon,
Rather than respond in detail to this and the other question you've
posted, I suggest you read up on threading in general. That way
hopefully you'll be able to come up with answers without us needing to
find out exactly what your situation is, etc.
In general you are right, but....

1)
I already have read a lot to generally understand what threads are about.
I now hear you say... "but obviously not enough.." ;-)
You are right again! On the other side I could spend years on reading about
threads without ever reaching the point that I can say :"Now I know all, no
more need to bother others...."
Thereby I woul probably never get done with my application....
This leads us to :

2.)
In some cases its just faster to ask a specific question befor....-> 1.)
.... forgive me ;-)
I've got a threading tutorial at
http://www.pobox.com/~skeet/csharp/threads

Hopefully that'll provide a good starting point for you.
Thanks for the link, I promise to read that too....(somewhen;) ...
especially the chapter on "
Exclusive access - Monitor.Enter/Exit and the lock statement
Regards
Rainer
 
R

Rainer Queck

Hi Mark,

thanks four your help. I thing now I got it!
Looks like it would in my case also make sence to use Queue<T>. I will
read.....

Thanks
Rainer
 
J

Jon Skeet [C# MVP]

Rainer said:
In general you are right, but....

1)
I already have read a lot to generally understand what threads are about.

Don't get me wrong - I wasn't trying to have a go at you. There was
nothing stupid about your question - it's just best answered at length,
in precise terms etc. Unfortunately, I don't have that much time right
now. Fortunately, I've had that much time in the past :)

Jon
 
R

Rainer Queck

Hi Jon,
Don't get me wrong
likewise ;-)
Unfortunately, I don't have that much time right
now.
I suffer the same problem. That's why "2.)" came into play.
Fortunately, I've had that much time in the past :)
Honestly! I am glad for that. I added your turorial to my KnowHow links, and
I hope to soon have the time to read it!

Regards
Rainer
 
N

Nick Hounsome

Rainer Queck said:
Hi Jon,

In general you are right, but....

1)
I already have read a lot to generally understand what threads are about.
I now hear you say... "but obviously not enough.." ;-)
You are right again! On the other side I could spend years on reading
about threads without ever reaching the point that I can say :"Now I know
all, no more need to bother others...."
Thereby I woul probably never get done with my application....

No - I'm afraid you are wrong - you clearly do not understand sufficiently
what thread s are about.

Threads is one subject above all others where a little knowledge is
dangerous thing - it can appear that you have done it correctly, the
compiler will accept it and your tests all work and then you do something as
simple as move to a multi CPU system and it will fails....unpredicatably,
unrepeatably and with no useful diagnostics after 3 months on a customer
site.

John is right you should read up about it some more.
If you understood it at all you wouldn't ask this question as it is virtualy
the "Hello World" of thread programming.
 
A

Anders Borum

Hello!
If you understood it at all you wouldn't ask this question as it is
virtualy the "Hello World" of thread programming.

Although the above statement may be right, I would still provide the OP a
few bonus points for posting the question here. Atleast it shows he's
interested in getting the right answer, and showing interest in doing
additional homework.
 
J

Jon Skeet [C# MVP]

Anders Borum said:
Although the above statement may be right, I would still provide the OP a
few bonus points for posting the question here. Atleast it shows he's
interested in getting the right answer, and showing interest in doing
additional homework.

Indeed. Besides, the question of how threadsafe a collection can be is
clearly not a totally simple one: I personally believe there's little
point in have "synchronized" versions of collections, as you so rarely
only need to perform one operation at a time. However, clearly people
at Microsoft (and Sun) disagree.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top