Last updated at 5:54 pm UTC on 31 October 2006
Ross Boylan September 21, 2005 Is Set growth thread-safe? My recent adventures led me to this code:
"Grow the elements array and reinsert the old elements"
| oldElements |
oldElements := array.
array := Array new: array size + self growSize.
tally := 0.
[:each | each == nil ifFalse: [self noCheckAdd: each]]
If another squeak thread (NOT native thread) interrupts this operation, isn't there a possibility for bad things to happen? This operation might be triggered any time an element is added to a set. A variation that constructed the new array first might be a bit safer, but it could still have problems if something was added or removed from the set in midstream. Subclasses of Set seem to have similar issues.
I thought Smalltalk was supposed to be thread-safe with respect to its own threads. Am I mistaken in that belief, or in my analysis that the operation is not thread safe? I'm aware that the solution would exact several kinds of penalties (clarity, speed, memory).
Daniel Vainsencher Squeak libraries are generally written non-thread safe. Where you intend to have interactions of multiple threads, you coordinate them, such as by using the specific thread safe data structures. Some of those are SharedQueue and the Transcript.
firstname.lastname@example.org I think you are mistaken in the belief [that 'Smalltalk was supposed to be thread-safe with respect to its own threads'.] AFAIK most classes are not thread safe at all, and not intended to be either. Then we have specific classes to deal with threads having to share objects - like Thread-safe, Monitor, SharedQueue, my SharedStreams package on SM etc.
Personally I think this is the "right way" to do it, no need to add complexity and overhead unless you know you need it. And typically I would say the objects shared by multiple Processes should be known and taken care for case by case. Or else you should use a transactional object memory - like say Magma/GemStone gives you
Bert Freudenberg It's not safe. You need to protect concurrent access to all objects that are not specifically designed for thread-safety (SharedQueue is thread-safe):
sema := Semaphore forMutualExclusion.
set := Set new.
"in threaded code"
sema critical: [set add: something].
Ross Boylan Thanks everyone for setting me straight. It seems reasonable that classes are not generally thread-safe, but in the original issue that got me into this, WeakIdentityKeyDictionary, it seems problematic.
WeakIdentityKeyDictionaries hold the system dependency information in Object's class variables DependentsFields and EventsFields. It is at least possible for a single application to serialize its use of the corresponding messages, but 2 separate applications in the same image have no way to do so. Even within a single application serializing access is challenging, given that read access must block while write access is happening.
Using the Model or EventsModel class clearly reduces the chances of problems, but the individual entries they employ are IdentityDictionaries and they may have similar issues (granted that the problem of multiple application coordination is now gone).
These issues might explain some erratic failures I've seen in my application. It has a background thread that does an update that triggers dependencies. So if the user is doing something interactively at the same time, bad stuff can happen. I tried to put appropriate guards in my code, but didn't realize the update mechanism itself needed protection.
In short, it seems too easy to shoot yourself in the foot with the present system design, unless the fact that threads are not pre-emptive provides some extra safety that I'm missing. By the usual logic of providing thread-safety around classes as needed, wouldn't it be better for Object, Model, and EventModel to protect the update mechanisms appropriately?
Bert Freudenberg No, ["it be better for Object, Model, and EventModel to protect the update mechanisms appropriately] because those mechanisms are intended to be used from the main UI thread. You should not send "changed" messages from a non-UI process, because they are handled synchronously (sending "update" messages
etc). You need to asynchronously let the UI process know it should act. There are several ways to accomplish this. A commonly used one in Morphic is this:
WorldState addDeferredUIMessage: [...]
You can call this from a background process, the block will later be executed in the UI process. Internally, it uses a SharedQueue holding the blocks.
Lex Spoon Thread-protecting the changed-update mechanism would seem like a reasonable thing, as would protecting the general event mechanism if it already isn't. These are small portions of code that are useful outside of the GUI thread. Instead of hardening the collection classes, though, it is likely much easier to add a Monitor to the class vars of class object and use that to protect critical regions.
Anyway, if it doesn't get fixed, a comment would seem in order, because we now have an existence proof of intelligent people being burned by this.
Oh, and along these lines, a motivated person might want to add a "Thread-Safe Libraries" page in Squeak saying what is (SharedQueue, Monitor, addDeferedUIMessage:) and isn't (almost everything) thread-safe in Squeak. IMHO, these pages are shaping up into a nice chunk of material for new Squeakers considering the use of threads, and are at the borderline of being required reading for serious Squeakers.
Squeak Threading Model