views:

98

answers:

7

I have class than spins off a backgroundworker to do some processor intensive stuff. The background worker reads a few strings that are declared globally for the whole class... do I need to lock around those strings? The backgroundworker never write the strings, they simply represent some directory locations that are set in the constructor of the class and are hardly ever written to by the class after the constructor (and never written to by the backgroundworker). So it's possible the background worker could read the string as it is also being written to by the main class object, though pretty unlikely. But wouldn't both those operations (the read by the background worker and the write by the main class) be atomic for a string literal anyway?

Thanks, -Robert

Edit: I don't care about the string being out of date or anything (that wouldn't be a big problem in my app), I'm more worried about getting the "object is in use elsewhere" exception.

+3  A: 

If you don't use a lock, the worst case would be that one of your background workers reads an outdated copy of the string from the perspective of your main class and thread. You will never (under any circumstance) encounter a "object in use elsewhere" exception when working with strings (from question).

As stated correctly in another answer, strings are immutable and cannot be changed after created. Any changes to an existing string will transparently result in a new string being created in memory on the heap, without any impact to the previous string object.

Using a lock (at the cost of a possibly measurable performance impact), will ensure that your background workers read the latest copy of the string.

Robert Venables
Using a lock really won't give any assurance that they background thread will get the latest. It would all be up to the scheduling of the threads.
Dolphin
@Dolphin: whenever a thread happens to run, it will never get old values of the data protected by the lock. It will always get the most recent (aka latest) value.
Martinho Fernandes
@Dolphin: The lock construct will create the appropriate memory barriers so yes it will guarentee that the latest value is read. Of course, if the writing thread did not use the same precaution then there is no guarentee that it's value would have been published, but the reading thread has no way of knowing that.
Brian Gideon
+6  A: 

Strings in .NET are immutable; they can't change. What happens is that the reference will point to a totally different string but the strings themselves won't be changed.

So if you don't particularly mind that the background workers might not all use the same string if you change it, then you should be fine. Example: Worker A reads string, something else changes it, Worker B reads new string—maybe this doesn't cause problems, maybe it does. But accessing the strings itself is definitely safe.

To quote from the documentation:

This type is thread safe.

ETA: A very good point by Martinho Fernandes in the comments below: Thread-safe objects to not automagically mean that everything you do with them is thread-safe as well. He even wrote a blog post on that which spares me the work of saying again everything he did :-)

Joey
This code is not thread-safe: `aString[aString.Length-1]`. Because strings are immutable it doesn't mean references to them are also immutable.
Martinho Fernandes
Good point, indeed.
Joey
Shameless plug: http://devnonsense.blogspot.com/2009/11/immutable-data-is-thread-safe.html
Martinho Fernandes
It's not shameless when it's perfectly relevant and helpful :)
JoeCool
Martinho: edited you in :-)
Joey
+1  A: 

If you are not doing any writing or modifying any shared variables then you don't need to use lock.

Stan R.
A: 

Yes, i would recomend locking, if it is such a small task as only reading, it should be fairly trivial. Just be aware of deadlock where a thread has locked a string and is waiting for another thread that has some other lock held.

astander
If it is such a small task as only reading, why lock?
Martinho Fernandes
A: 

You don't need the lock since such a race would only occur if two threads attempt to assign a value concurrently. Since .NET strings are immutable, the result you get is never corrupt - in the worst case, it's outdated.

Dario
+1  A: 

There are several strings A B & C? Does it matter if the background worker is working on (say) v3 of A and B and v2 of C? If so then you need a lock around the update of the whole set.

A second subtle problem is that C# might choose cache values in registers or otehrwise reorder your code and so the threads don't see the same view of "reality". See this discussion and answers to this SO question.

My recommendation, write the conspicuously correct code using synchronisation. In this scenario the performance impact is surely trivial. That way the code maintainer doesn't even need to worry. If benchmarking reveals this to be a performance problem then be very, very careful as you study writing thread-safe clever code.

djna
+2  A: 

Yes, reads and writes will be atomic to string variables. That is because only the variable reference is ever changed. Strings are immutable so any operation that modifies the contents of the string will also create a new instance of the string. It is the reference to that new instance that is swapped out via the variable. But, that is not the main issue.

The main issue has to do with the staleness of the string variable itself. Without the appropriate synchronization mechanisms the writes in one thread may not be seen in another thread.

Bottom line...if there is even a remote chance that another thread will modify the string variable then you will need to synchronize access to it from the worker thread and most likely your main thread as well.

Edit: Since staleness is of no concern to you then you will probably be okay without using locks. However, the assumption is that you have initialized the string variable to something before the worker thread starts.

Brian Gideon