TraceListenerCollection is not thread-safe

[CurtHagenlocher]

Both CoreFx and the desktop implementations claim that TraceListenerCollection is thread-safe. But two threads executing System.Diagnostics.Trace.Listeners.Remove("Default") at the same time can run into each other. The problem is that several operations (like Remove(string)) are built on top of enumerating the collection, and enumerating the collection will neither lock it nor make a copy of it.

It's possible to squint and say "but it is thread-safe in that its internal state can't be corrupted via multiple simultaneous mutation", but I don't think that's consistent with the way that most people think of thread-safety. If nothing else, the documentation should be updated.

[karelz]

Note: This is API only for compat. We don't plan to evolve it too much - use EventSource instead. If someone wants to tackle it anyway:

Next step: Propose a change to code. Complexity: Simple-Medium (maybe just add some lock somewhere)

[jacobjmarks]

I'd be unsure as to how this is occurring as TraceListenerCollection does contain a lock around all of its internal list manipulation[^1], including Remove:

[^1]: Perhaps bar the set operation on the int indexer (reference).

https://github.com/dotnet/runtime/blob/830d3d009091a5f5a5260fafd6649f87892f8c4a/src/libraries/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceListeners.cs#L288-L294

[CurtHagenlocher]

Well, it's been a few years of course but from what I remember I discovered this running against Desktop framework and then checked the published Core implementation and saw that it seemed to be the same. Apparently it's either been fixed since then or I misread the Core implementation at the time. I'll close this item.

The Desktop implementation was never changed if you want to see what it looked like back then.