Synchronized Collection<T> Recommendation

M

Michael Primeaux

What is the recommended pattern for implementing a synchronized
(thread-safe) class that inherits from Collection<T>? For example, I want
to implement a SyncRoot property [of type
System.Threading.ReaderWriterLock]. I do see where I can override
(protected) the methods InsertItem, RemoveItem, ClearItems, and SetItem.
However, I do not see an override for GetItem.

Kindest regards,
Michael
 
C

Chris Chilvers

What is the recommended pattern for implementing a synchronized
(thread-safe) class that inherits from Collection<T>? For example, I want
to implement a SyncRoot property [of type
System.Threading.ReaderWriterLock]. I do see where I can override
(protected) the methods InsertItem, RemoveItem, ClearItems, and SetItem.
However, I do not see an override for GetItem.

Kindest regards,
Michael

You could inherit from ICollection<T> and write a synced wrapper that
can accept any ICollection<T> and make all the methods syncronized. Then
you can write a standard non synced collection and wrap it in a synced
one?

For one thing you cannot garuntee that at some point some method such as
FastAdd(T[] array) doesn't get added that instead of calling SetItem
calls directly into the underling datastore for speed. Now you have an
unsyncronized method. With a wrapper this wouldn't happen due to the
fact that the FastAdd method would be hidden. If you wanted to use you'd
have to explicitly add access to it and can again ensure it is synced.

Infact Collection<T> is just a wrapper for IList<T> so you could write
an IList<T> wrapper that syncronizes an IList<T> then pass this
syncronized list into Collection<T>:

List<T> baseList = new List<T>();
SyncronizedList<T> syncedList = new SyncronizedList<T>(baseList);
Collection<T> col = new Collection<T>(syncedList);

Unfortunatly they seem to have dropped the IsSyncronized property from
the ICollection<T> interface, so there is no way to tell if a generic
collection is already syncronized so that you only have to wrap it if it
isn't. This is a shame as it means that Collection<T>.IsSyncronized
always returns false even if the inner list is infact syncronized.
 
M

Michael Primeaux

Chris. Thanks for your reply. I've read many blogs and articles on "why"
Microsoft didn't--and I quote--"make the same mistake" as they did in .NET
1.1 regarding inclusion of the SyncRoot and IsSynchronized properties in the
..NET 2.0 framework.

IMHO, I disagree with Microsoft's reasoning:
http://blogs.msdn.com/brada/archive/2003/09/28/50391.aspx and do certainly
find use for the SyncRoot and IsSynchronized properties in multi-operation
scenarios (i.e. add one item and remove another in the same protected
operation).

Unless I'm missing something, it seems as though not allowing override of
item retrieval on Collection<T> is an oversight. Granted I could use the
"new" keyword on the Collection<T> indexer but that doesn't seem as elegant.

Again, I appreciate your reply.

- m
Chris Chilvers said:
What is the recommended pattern for implementing a synchronized
(thread-safe) class that inherits from Collection<T>? For example, I want
to implement a SyncRoot property [of type
System.Threading.ReaderWriterLock]. I do see where I can override
(protected) the methods InsertItem, RemoveItem, ClearItems, and SetItem.
However, I do not see an override for GetItem.

Kindest regards,
Michael

You could inherit from ICollection<T> and write a synced wrapper that
can accept any ICollection<T> and make all the methods syncronized. Then
you can write a standard non synced collection and wrap it in a synced
one?

For one thing you cannot garuntee that at some point some method such as
FastAdd(T[] array) doesn't get added that instead of calling SetItem
calls directly into the underling datastore for speed. Now you have an
unsyncronized method. With a wrapper this wouldn't happen due to the
fact that the FastAdd method would be hidden. If you wanted to use you'd
have to explicitly add access to it and can again ensure it is synced.

Infact Collection<T> is just a wrapper for IList<T> so you could write
an IList<T> wrapper that syncronizes an IList<T> then pass this
syncronized list into Collection<T>:

List<T> baseList = new List<T>();
SyncronizedList<T> syncedList = new SyncronizedList<T>(baseList);
Collection<T> col = new Collection<T>(syncedList);

Unfortunatly they seem to have dropped the IsSyncronized property from
the ICollection<T> interface, so there is no way to tell if a generic
collection is already syncronized so that you only have to wrap it if it
isn't. This is a shame as it means that Collection<T>.IsSyncronized
always returns false even if the inner list is infact syncronized.
 
W

William Stacey [MVP]

| IMHO, I disagree with Microsoft's reasoning:
| http://blogs.msdn.com/brada/archive/2003/09/28/50391.aspx and do certainly
| find use for the SyncRoot and IsSynchronized properties in multi-operation
| scenarios (i.e. add one item and remove another in the same protected
| operation).

I have to agree with MS on this one. To add an item and remove in the same
protected operation means your taking the lock 3 times. Once for the
lock(SyncRoot), and once for each method internally. If you add a .Count in
there, then its four locks. True, you already own the lock, so the other
lock operations are faster, but it is still wasted overhead. As a user of
the collection, it often cleaner, and faster, to just use your own lock and
sync access to the collection yourself. Plus you may need to leverage that
lock for other issues outside of the collection.
 
M

Michael Primeaux

That is true if I were to use lock / Monitor. I prefer to use the
ReaderWriterLock class in more cases (especially for the SyncRoot property)
for obvious reasons; I can't know (or presume to know) the usage profile of
the consumer.

I'll still keep my opinion; but then again, you know what those are like ;-)
 
M

Michael Primeaux

Hit SEND too soon.

That is true if I were to use lock / Monitor. I prefer to use the
ReaderWriterLock class in more cases (especially for the SyncRoot property)
for obvious reasons; I can't know (or presume to know) the usage profile of
the consumer.

The purpose of the SyncRoot property IS to leaverage the lock for other
scenarios outside of the collection but as related TO the collection.

I'll still keep my opinion; but then again, you know what those are like ;-)
 
C

Chris Chilvers

Looking at the reasoning it does make sense to me, if you want something
to be thread safe you shouldn't allow access to (by passing around) an
unsynched version and require that the callee does the appropriate
locking.

Collection<T> is just really a wrapper for IList<T>, seems like it was
only really put in to provide a simple wrapper to make custom
implementations of IList<T> use the old ICollection, IList, and
IEnumerable interface with out having to implement them by hand. Thus it
appears to me that it is not designed to be overrode, and instead the
item that should be overrode or wrapped is the IList being passed into
the Collection<T>.

Might be best just to have two synced wrappers:
SyncedList<T> : IList<T>
SyncedCollection<T> : ICollection<T>


Chris. Thanks for your reply. I've read many blogs and articles on "why"
Microsoft didn't--and I quote--"make the same mistake" as they did in .NET
1.1 regarding inclusion of the SyncRoot and IsSynchronized properties in the
.NET 2.0 framework.

IMHO, I disagree with Microsoft's reasoning:
http://blogs.msdn.com/brada/archive/2003/09/28/50391.aspx and do certainly
find use for the SyncRoot and IsSynchronized properties in multi-operation
scenarios (i.e. add one item and remove another in the same protected
operation).

Unless I'm missing something, it seems as though not allowing override of
item retrieval on Collection<T> is an oversight. Granted I could use the
"new" keyword on the Collection<T> indexer but that doesn't seem as elegant.

Again, I appreciate your reply.

- m
Chris Chilvers said:
What is the recommended pattern for implementing a synchronized
(thread-safe) class that inherits from Collection<T>? For example, I want
to implement a SyncRoot property [of type
System.Threading.ReaderWriterLock]. I do see where I can override
(protected) the methods InsertItem, RemoveItem, ClearItems, and SetItem.
However, I do not see an override for GetItem.

Kindest regards,
Michael

You could inherit from ICollection<T> and write a synced wrapper that
can accept any ICollection<T> and make all the methods syncronized. Then
you can write a standard non synced collection and wrap it in a synced
one?

For one thing you cannot garuntee that at some point some method such as
FastAdd(T[] array) doesn't get added that instead of calling SetItem
calls directly into the underling datastore for speed. Now you have an
unsyncronized method. With a wrapper this wouldn't happen due to the
fact that the FastAdd method would be hidden. If you wanted to use you'd
have to explicitly add access to it and can again ensure it is synced.

Infact Collection<T> is just a wrapper for IList<T> so you could write
an IList<T> wrapper that syncronizes an IList<T> then pass this
syncronized list into Collection<T>:

List<T> baseList = new List<T>();
SyncronizedList<T> syncedList = new SyncronizedList<T>(baseList);
Collection<T> col = new Collection<T>(syncedList);

Unfortunatly they seem to have dropped the IsSyncronized property from
the ICollection<T> interface, so there is no way to tell if a generic
collection is already syncronized so that you only have to wrap it if it
isn't. This is a shame as it means that Collection<T>.IsSyncronized
always returns false even if the inner list is infact syncronized.
 
W

William Stacey [MVP]

| That is true if I were to use lock / Monitor. I prefer to use the
| ReaderWriterLock class in more cases (especially for the SyncRoot
property)
| for obvious reasons;

Same holds true for ReaderWriteLock. RW may, in fact, not gain you anything
but slower performance. Is your collection doing anything besides updating
a list? If not, then a monitor will most likely be factors faster.

| I can't know (or presume to know) the usage profile of
| the consumer.

Exactly. You can't know, so why implement it? It is a simple matter for
the user to do it as they know what they need.

| I'll still keep my opinion; but then again, you know what those are like
;-)

:)
 
M

Michael Primeaux

Given the current implementations of Collection<T> and List<T>, I agree in
that two synchronized wrappers are the correct approach. I'll proceed along
this path until such time as other possibilities arise.

Again, I do appreciate your time.

Chris Chilvers said:
Looking at the reasoning it does make sense to me, if you want something
to be thread safe you shouldn't allow access to (by passing around) an
unsynched version and require that the callee does the appropriate
locking.

Collection<T> is just really a wrapper for IList<T>, seems like it was
only really put in to provide a simple wrapper to make custom
implementations of IList<T> use the old ICollection, IList, and
IEnumerable interface with out having to implement them by hand. Thus it
appears to me that it is not designed to be overrode, and instead the
item that should be overrode or wrapped is the IList being passed into
the Collection<T>.

Might be best just to have two synced wrappers:
SyncedList<T> : IList<T>
SyncedCollection<T> : ICollection<T>


Chris. Thanks for your reply. I've read many blogs and articles on "why"
Microsoft didn't--and I quote--"make the same mistake" as they did in .NET
1.1 regarding inclusion of the SyncRoot and IsSynchronized properties in
the
.NET 2.0 framework.

IMHO, I disagree with Microsoft's reasoning:
http://blogs.msdn.com/brada/archive/2003/09/28/50391.aspx and do certainly
find use for the SyncRoot and IsSynchronized properties in multi-operation
scenarios (i.e. add one item and remove another in the same protected
operation).

Unless I'm missing something, it seems as though not allowing override of
item retrieval on Collection<T> is an oversight. Granted I could use the
"new" keyword on the Collection<T> indexer but that doesn't seem as
elegant.

Again, I appreciate your reply.

- m
Chris Chilvers said:
On Sun, 30 Apr 2006 09:45:40 -0500, "Michael Primeaux"

What is the recommended pattern for implementing a synchronized
(thread-safe) class that inherits from Collection<T>? For example, I
want
to implement a SyncRoot property [of type
System.Threading.ReaderWriterLock]. I do see where I can override
(protected) the methods InsertItem, RemoveItem, ClearItems, and SetItem.
However, I do not see an override for GetItem.

Kindest regards,
Michael


You could inherit from ICollection<T> and write a synced wrapper that
can accept any ICollection<T> and make all the methods syncronized. Then
you can write a standard non synced collection and wrap it in a synced
one?

For one thing you cannot garuntee that at some point some method such as
FastAdd(T[] array) doesn't get added that instead of calling SetItem
calls directly into the underling datastore for speed. Now you have an
unsyncronized method. With a wrapper this wouldn't happen due to the
fact that the FastAdd method would be hidden. If you wanted to use you'd
have to explicitly add access to it and can again ensure it is synced.

Infact Collection<T> is just a wrapper for IList<T> so you could write
an IList<T> wrapper that syncronizes an IList<T> then pass this
syncronized list into Collection<T>:

List<T> baseList = new List<T>();
SyncronizedList<T> syncedList = new SyncronizedList<T>(baseList);
Collection<T> col = new Collection<T>(syncedList);

Unfortunatly they seem to have dropped the IsSyncronized property from
the ICollection<T> interface, so there is no way to tell if a generic
collection is already syncronized so that you only have to wrap it if it
isn't. This is a shame as it means that Collection<T>.IsSyncronized
always returns false even if the inner list is infact syncronized.
 
M

Michael Primeaux

Exactly. You can't know, so why implement it?
The answer is easy; by using lock you presume to know the usage profile of
the consumer is one that doesn't favor a higher read frequency. By using the
ReaderWriterLock class, you favor either choice.
Is your collection doing anything besides updating a list?
Again, by assuming an answer here you presume to know the usage profile of
the consumer of the collection. In this case, you assume the to know the
consumer favors reads equally as they favor writes. Regardless of whether my
collection only updates a "list", Monitor forces threads to take it in
turns, while the ReaderWriterLock only serializes access when writes occur.
A properly written collection (or framework API in general) doesn't assume
to assess the contention model of protected resources. Stated another way;
as the write frequency approaches the read frequency then the use of the
ReadWriterLock versus Monitor becomes moot but ONLY as related to
concurrency rate.
 
W

William Stacey [MVP]

| The answer is easy; by using lock you presume to know the usage profile of
| the consumer is one that doesn't favor a higher read frequency. By using
the
| ReaderWriterLock class, you favor either choice.

I think I said, don't assume anything. Let the consumer decide.

| > Is your collection doing anything besides updating a list?
| Again, by assuming an answer here you presume to know the usage profile of
| the consumer of the collection. In this case, you assume the to know the
| consumer favors reads equally as they favor writes. Regardless of whether
my

I was trying not assume, that is why I asked the question. What does your
collection do? As the answer can help dictate the lock to use.
ReaderWriter is not free. It is factors slower then a single monitor, so I
would just not use it everywhere as an alternative to monitor. Plus a RW,
can be unfair to writers (which may or may not be an issue for you). If
your readers are processing or blocking a long time, then a RW can be a good
thing. If not, on a single cpu it will not even help. On a multi-cpu it
may help a little. But if you are in and out of the read lock fast (i.e.
few non-blocking instructions) then the odds are there will not be
contention for the lock anyway and you only increase the odds of contention
using the slower lock (over a monitor). Naturally, testing perf is the only
way to know for sure.
 
C

Chris Chilvers

All this locking will however slow things down for such things as
for (int i = 0; i < list.Count; i++) ...

Which will now lock every time it reads something, and what if:

Thread A increments i to the last element (count-1)
Thread B deletes the last element
Thread A tries to read the last element

In these cases I'd favor having an unsyschronized collection and having
some specified way to obtain the collection exclusivly.

It just seems to me that syncronization on such things as a collection
are very much dependent upon what the collection is storing and what the
data is being used for. Without knowing the specific case it will be
used it just seems like one cannot reliably choose the locking mechanism
or guarantee that it will even work correctly as shown in the previous
example.
 
M

Michael Primeaux

Regarding the Thead A, B, A scenario below, ultimately it's the
responsibility of the developer to ensure Thead A holds a lock until its
operations are complete. I mean, that's multi-threaded programming 101. I
can see an argument that assumes I hold a reference to an object (contained
in a collection) and that I set that reference equal to a new object
instance without first aquiring ownership of a synchronization primitive.
Effectively, I just updated the collection but without first aquiring a
lock. Again, you can't protect the developer from everything but then again
the developer had better know what they're doing. Ironically, this is the
very scenario that having a SyncRoot property addresses.

The intent of my original email was to inquire as to the recommended
solution for implementing a thread-safe collection that inherits from
Collection<T>. I certainly understand why the IsSynchronized property always
returns false and the SyncRoot property returns the current instance. I do
agree with this approach as the earnest is on the consumer to ensure
"proper" locking. I'm really just curious as to why no overridable members
exist for item retrieval on the Collection<T> class.
 
M

Michael Primeaux

William,

I think you mean to say only scalability testing (and not performance) is
the only way to know for sure. Performance testing is usually done with a
small number of users whereas concurrency rates are effectively measured
during scalability testing. My only point is that by using lock/Monitor you
prevent the consumer from deciding because you're making a statement that
they have a near equal read and write frequency. Using the ReaderWriterLock
class adjusts to the consumer's usage profile. For example, if the
application has a higher read frequency than write frequency then--assuming
an equal contention rate--the ReaderWriterLock class on a multi-CPU machine
will *more than likely* result in a higher concurrency rate. If the
application's read and write frequency are near identical then you might as
well use lock/Monitor.

You asked what my collection does. It's as generic in usage as the
Collection<T> class. The only difference is I want to return a
ReaderWriterLock class for the SyncRoot property.

Kindest regards,
Michael
 
J

Jon Skeet [C# MVP]

Michael Primeaux said:
I think you mean to say only scalability testing (and not performance) is
the only way to know for sure. Performance testing is usually done with a
small number of users whereas concurrency rates are effectively measured
during scalability testing. My only point is that by using lock/Monitor you
prevent the consumer from deciding because you're making a statement that
they have a near equal read and write frequency. Using the ReaderWriterLock
class adjusts to the consumer's usage profile. For example, if the
application has a higher read frequency than write frequency then--assuming
an equal contention rate--the ReaderWriterLock class on a multi-CPU machine
will *more than likely* result in a higher concurrency rate. If the
application's read and write frequency are near identical then you might as
well use lock/Monitor.

Have you actually benchmarked this to find out how many concurrent
readers (or for how long) you require before it's worth using
ReaderWriterLock? I haven't use RWL myself, but I've read (from people
I trust) that it's sufficiently slow as to make it worthless in all but
the most extreme situations.
You asked what my collection does. It's as generic in usage as the
Collection<T> class. The only difference is I want to return a
ReaderWriterLock class for the SyncRoot property.

Ah - this is slightly different. Does this mean you won't actually be
doing any of the locking yourself in the first place? If so, that's
okay, as you won't be *forcing* anyone to use a ReaderWriterLock when
they don't want one, even if they do want thread safety.
 
H

Helge Jensen

Michael said:
What is the recommended pattern for implementing a synchronized
(thread-safe) class that inherits from Collection<T>? For example, I want
to implement a SyncRoot property [of type
System.Threading.ReaderWriterLock]. I do see where I can override
(protected) the methods InsertItem, RemoveItem, ClearItems, and SetItem.
However, I do not see an override for GetItem.

I have an issue with the whole business of synchronized collections
altogether. Using or changing a collection usually requires it to be
user-controlled temporally immutable across invocations of methods anyway.

For me, synchronization of methods on collections is mostly an
*implmentation* issue, which an implementer can use to prevent the
data-structure from corruption from multiple threads operating on the
structure at once.

Locking for iteration and actions depending on containment, cannot
usually be solved by synchronized methods, it requires block-level
locking to guarantee synchronization between invocations of methods:

lock ( collection ) { // or collection.SyncRoot in .NET1
int count = collection.Count;
foreach ( object t in collection )
...;
}
lock ( collection ) {
if ( t.Contains(x) )
...;
}


WRT, SyncRoot:

I can't understand the "lock-counting" arguments, I don't see that they
come into the discussion at all. It's much more important whether there
is a protocol for passing a collections mutability around. That said,
note that if the client properly locks before using a collection there
is *one* lock-attempt for each "conversation" between a client and a
collection.

In .NET1, I had lots of adapters for collections, which would simply
pass their inner.SyncRoot for their own SyncRoot. In .NET2, it now
requires an external protocol to make that work.

It's not that I'm too glad of SyncRoot, but I was glad *something* was
there.

SyncRoot only allowed lock/Monitor. I would prefer a protocol where you
could allocate Read and/or Write permission, and obtain an object which
would guarantee you that permission, and associated guaratees on
synchonization of read and writes, untill you Dispose() that object.
 
C

Christoph Nahr

Locking for iteration and actions depending on containment, cannot
usually be solved by synchronized methods, it requires block-level
locking to guarantee synchronization between invocations of methods:

Precisely. I've always said that the Synchronized wrappers and
IsSynchronized flags in .NET 1.x collections were a terrible idea.
Developers who weren't 100% familiar with threading issues would use a
Synchronized wrapper ("oh, the docs say it's thread-safe!"), then do a
multi-step read/write sequence in concurrent threads -- bang, data
corruption ensues. And if they remembered to lock manually those
wonderful overriden locking methods were completely useless.

On the other hand, I don't see how this applies to the SyncRoot
property as Brad Abrams claimed. SyncRoot is just an object
associated with the collection that can be used for locking. Yeah,
you need multiple locks on different SyncRoots if you manipulate
multiple collections in one block but that's still not a big deal IMO.
 
W

William Stacey [MVP]

| On the other hand, I don't see how this applies to the SyncRoot
| property as Brad Abrams claimed. SyncRoot is just an object
| associated with the collection that can be used for locking. Yeah,
| you need multiple locks on different SyncRoots if you manipulate
| multiple collections in one block but that's still not a big deal IMO.

That IMO, is a reason why they removed it. Normally a collection writer
using containment will already have a syncroot object for the class -
exposed or not. And use that object to sync all invariants in the
collection (maybe even abstracting multiple collections). So the SyncRoot
Property in the contained collections are never used and become "overhead"
in the API. If there is no sync wrapper using it, there is no need for a
SyncRoot property.
 

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