Do I need a critical section to get the index of a value in a stringlist?

Question

I have a stringlist myStringList with abouth 100 values and I'm doing asynchronous access to it.I would like to know if it is thread safe to do this :

currentIndex := myStringList.IndexOf(wantedValue);

or I always have to do this :

criticalS.Enter;
try
  currentIndex := myStringList.IndexOf(wantedValue);
finally
  criticalS.Leave;
end;

Duhon

Your second snippet is kind of useless. Serializing the access with a critical section may protect against concurrent modifcations to the string list, but the returned currentIndex may be invalid by the time the thread has executed the criticalS.Leave operations. What could it ever be used for? Instead, try to define whole operations on your shared data, and protect those whole operations with locking. A public interface like that of TStrings is completely useless in a concurrent scenario. Of course, if you don't have concurrent modifications the first is completely sufficient. – Engrave

Answer 1

You absolutely do need to use a lock to protect all access of a mutable string list. If a writer thread modifies the list whilst your thread reads, the code can fail. You might refer to a string that has been destroyed. Or the list may be reallocated to a different address whilst you are reading it.

You need to use the same lock for all access, both read and write. You can use a multiple read, exclusive write lock rather than a critical section. But you do need synchronisation of some form.

If all threads are reading from the list, and no thread modifies it in any way, then you do not need a lock.

Hebron

I'm filling myStringList at the beginning of the program with some values and then it stays the same till the end of execution - I only access it's data in order to get the index of a value I need - from what you're saying I don't need critical section to read from myStringList ? – Duhon

Correct. So long as the list is static once you commence parallel access then you do not need to lock. – Hebron

It might still not be safe, because of reference counting when accessing the strings stored in the list. If multiple threads access the same string, the reference count might end up too high or too low, resulting in an AV later. – Thermopylae

@Thermopylae Reference counts are only modified when the string list contents are being modified. Reading the value does not result in reference count modification. What's more, reference counts are managed using atomic increments and so reference counts never end up incorrect due to a race. Of course, there are race issues, but not the way you tell it, and certainly no race issues if you don't modify the strings. – Hebron

@DavidHeffernan That's wrong: Reference counts get incremented on read access. A write access will create a copy of the string and then decrement the reference count of the original string. I'm not sure about the reference counts being atomic operations, haven't looked at that code yet. – Thermopylae

@Thermopylae No you are right about reading causing ref count changes. Silly me. But ref counting is atomic to avoid the very problem. So the scenario in the Q, static list, is fine for unprotected parallel access. – Hebron

@DavidHeffernan looks like you are right. The code in _LStrAsg and _LStrClr seems to use locking. Whether it is correct code I don't dare to say, but I have some faith in the RTL developers. – Thermopylae

Answer 2

Here's a very nice thread-safe TStringList, which I've used very successfully.

TThreadStringList by Tilo Eckert http://www.swissdelphicenter.ch/torry/showcode.php?id=2167

It demonstrates the critical section technique very clearly.

Anarch

I would tune that code up with the InitializeCriticalSectionAndSpinCount critical section. Btw. even though we know that OP has a consistent list during his app. lifetime and thus doesn't need to use a lock in his example, it's worth to mention that Indy ships with TIdThreadSafeStringList. – Pishogue

@TLama, interesting! I will definitely add it to mine. – Anarch

Answer 3

If you do not manipulate with strings (add/insert/remove) from any other thread (meaning, data is there and it's static), there is no need for lock.

Boddie