[Mono-dev] [PATCH] ReadOnlyCollection<T>

[Juraj Skripsky]

Hello Kosta,

Thanks for the feedback. You're right, the code is not thread-safe. I've
changed the SyncRoot getter to return "this" as specified in the docs. 

The updated patch is attached.

- Juraj


On Fri, 2008-01-18 at 07:38 -0800, Konstantin Triger wrote:
> Hey Juraj,
> 
> The following code:
> 
> +               // LAMESPEC: According to the docs, this should return
> "this".
>                 object ICollection.SyncRoot {
> -                       get { return syncRoot; }
> +                       get { return syncRoot ?? syncRoot = new
> object(); }
>                 }
> 
> is thread unsafe. Thus you may end up with 2 threads syncing on 2
> different object.
> 
> If you want to use a class field as a SyncRoot you should declare it
> readonly to comply with all the possible CPU architectures and VM
> implementations.
> 
> BTW, even if your implementation would correct, I don't see any
> advantage over simple "return this;", even if tests show otherwise.
> 
> Regards,
> Kosta
> 
> 
> 
> -----Original Message-----
> From: mono-devel-list-bounces at lists.ximian.com on behalf of Juraj
> Skripsky
> Sent: Fri 1/18/2008 14:34
> To: mono-devel-list at lists.ximian.com
> Subject: [Mono-dev] [PATCH] ReadOnlyCollection<T>
> 
> Hello,
> 
> Attached you'll find a patch and new unit tests for
> ReadOnlyCollection<T>. All unit tests pass on Mono and .NET.
> 
> Please review.
> 
> - Juraj
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ROColl.patch
Type: text/x-patch
Size: 3667 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20080118/6ed430d7/attachment.bin