Is .NET's StringBuilder thread-safe

Question

The regular "Thread Safety" section of the MSDN documentation for StringBuilder states that:

...any instance members are not guaranteed to be thread safe...

but this statement feels like it has been copied and pasted for almost every class in the Framework:

http://msdn.microsoft.com/en-us/library/system.text.stringbuilder.aspx

However, these blog posts by Gavin Pugh mention thread-safe behaviours of StringBuilder:

http://www.gavpugh.com/2010/03/23/xnac-stringbuilder-to-string-with-no-garbage/

http://www.gavpugh.com/2010/04/01/xnac-avoiding-garbage-when-working-with-stringbuilder/

Additionally, the source of StringBuilder revealed by Reflector, and the accompanying comments in the SSCLI source, also suggest many implementation considerations to ensure thread-safety:

http://labs.developerfusion.co.uk/SourceViewer/browse.aspx?assembly=SSCLI&namespace=System.Text&type=StringBuilder

Does anyone have any more insight into whether a StringBuilder instance is safe to share among multiple concurrent threads?

Answer 1

Absolutely not; here's a simple example lifted from 4.0 via reflector:

[SecuritySafeCritical] public StringBuilder Append(char value) {     if (this.m_ChunkLength < this.m_ChunkChars.Length)     {         this.m_ChunkChars[this.m_ChunkLength++] = value;     }     else     {         this.Append(value, 1);     }     return this; } 

The attribute just handles callers, not thread-safety; this is absolutely not thread-safe.

Update: looking at the source he references, this is clearly not the current .NET 4.0 code-base (comparing a few methods). Perhaps he is talking about a particular .NET version, or maybe XNA - but it is not the case in general. The 4.0 StringBuilder does not have a m_currentThread field, which Gavin's source material uses; there's a hint (an unused constant ThreadIDField) that it used to exist, but... no longer.

---

If you want a direct disproof - run this on 4.0; it will most likely give the wrong length (I've seen a few in the 4k region, a few in the 2k region - it should be exactly 5000), but some other Append methods (Append(char) for example) tend more likely to throw exceptions, depending on timing:

var gate = new ManualResetEvent(false); var allDone = new AutoResetEvent(false); int counter = 0; var sb = new StringBuilder(); ThreadStart work = delegate {     // open gate when all 5 threads are running     if (Interlocked.Increment(ref counter) == 5) gate.Set();     else gate.WaitOne();      for (int i = 0; i < 1000; i++) sb.Append("a");      if (Interlocked.Decrement(ref counter) == 0) allDone.Set(); }; for(int i = 0 ; i < 5 ; i++) {     new Thread(work).Start(); } allDone.WaitOne(); Console.WriteLine(sb.Length); 

Answer 2

The whole point of the documentation is to give you guarantees. In this case on instance members nothing is guaranteed to be thread-safe and you should treat it as such, therefore relying on external synchronisation methods.

That some things may be threadsafe is an implementation detail which can and maybe does change from one version of the framework to the next or from one implementation to the next (in fact there are plenty of such details changing in framework versions; Eric Lippert has some posts detailing a few of them). Don't rely on it.

(In other words: Don't write code to an implementation, write it against the interface and contract which is the metadata of the class and its documentation in this case.)