In order to properly fix
It turned out that it would be convenient for Condition to not allow duplicate entries to the same thing in its waitingThreads, so e.g. use an IdentitySet instead of the Array it presently uses. Can someone think of a reason why this is a bad idea, i.e. any reason to allow duplicate rescheduling of the same thing in Condition? (The newer CondVar also allows duplicates, by the way.)
Actually, I can think of one myself, although it was not part of the original SC3 design. threadPlayer does scheduler activation stealing by substituting the same “parent” object for multiple threads in those Condition queues.
So, it looks like a real solution would have to combine something else I was working on (for a different reason), namely Condition queues that keep pairs of [threadPlayer, thread] as queue elements. So those can be deduplicated, i.e. only one pair of unique [threadPlayer, thread] per queue makes sense to me. Come to think of it, the structure can be an IdentityDictionary that maps the original thread to its activation-stealing threadPlayer. I can’t quite think of a reason now to allow multiple threadPlayers to own the same thread. The way the parent-based lookup is done in Thread.findThreadPlayer precludes that, although one can build non-tree graphs bypassing that lookup.
It is a list because the same thread can’t be waiting twice in the same condition, if that’s the case something is wrong elsewhere. I think there might be a misunderstanding about what threadPlayer is for, if you see findThreadPlayer that gives you a definition: is the outermost thread that is not the main thread or self. Note: for every thread you have the thread and the threadPlayer, there is no need to save both separately. It is not possible to change the parent hierarchy, that’s defined when routines next call are nested. It is not the same case when a routine is played in a clock from within another routine, the next method is called from a clock, from the main thread. For simple usage of EventStreamPlayer and other cases, the threadPlayer is the routine actually playing in a clock.
In the cooperative model of SC’s threads and yield-based “blocking” (which is not really blocking, just de-scheduling) it sure can be waiting twice, because it can be rescheduled by any other thread calling it again after it supposedly “blocked”, which really just means it yielded “hang” as opposed yielding 42 or whatever else. See example on the linked github page.
Threads, conditions, and promises/flowvars aren’t the same as in other languages, threads are not like system threads, conditions are not locks, flowvars don’t use an inner/hidden event loop.
Yeah, ok, so? Doesn’t that (1) repeat what I said and (2) invalidate your previous point?
Ok, then (1) you misunderstand most of the code logic (2) suit yourself.
I suspect (though I can’t prove) that it’s a mistake (i.e. wrong usage) If a thread 1/ blocks in a CondVar, 2/ unblocks by some other means, 3/ re-blocks in the same CondVar (???).
It’s probably OK to prevent that.
I wouldn’t use IdentitySet because CondVar signalOne depends on the order of the collection. (However, here might actually be a potential use case for duplicates…? Though it still seems weird to me.)
BTW I keep saying CondVar in response to Condition because Condition is out of date. We really should deprecate it, but haven’t yet. Part of the history is that somebody tried to do timeouts by condition.hang(aNumber), but this doesn’t really work because the timeout will fire even if the condition unblocked earlier… but it persisted for years as folklore that “we have timeouts.” IMO we should encourage the use of CondVar instead.
Agreed, there is no reason to allow that.
There’s a certain degree of unfocused speculation in this whole inquiry. It’s like a tree traversal where there are no leaf nodes – we could keep talking about this forever, and never get anything done. Would it perhaps be more productive to define the boundaries within which thread usage makes sense in SC? And then declare the rest to be wrong usage (undefined behavior, at the user’s own risk etc.).
threadPlayer exists for one purpose: to link a Routine to its owner PauseStream/Task/EventStreamPlayer. If users hijack this for other purposes, it’s wrong usage and we have no obligation to support that.
It’s nonsense for one Routine to be controlled by multiple PauseStreams. You can do it, but nobody actually does. So to some extent, this is responding to a hypothetical problem but not one that seriously impacts anyone.
hjh
Only if you want to guarantee FIFO execution of blocked (.wait) callers.
To me, that is more logical behavior than “signalOne will awaken any randomly chosen waiting thread, regardless of the order in which they were paused.” If I encountered the latter in documentation, I would wonder what is the scenario in which it would be good to signalOne and be uncertain what would happen next.
hjh