_BACKGROUND_
In relation to...
https://pharo.fogbugz.com/default.asp?13182
"Refactor MCFileRepositoryInspector >> versionSearchAccept:"
I am reviewing...
https://pharo.fogbugz.com/default.asp?13007
"Cannot integrate a slice with monticello (red square in the UI)"
and in general considering how Monticello uses multiple threads to allow
you to type in the filter while its downloading info from the remote
repository (which would be good to have more of this sort thing).
Looking back at build 30814 just prior to slice 13007 being integrated,
I notice that MCFileRepositoryInspector>>setRepository:workingCopy: [1]
forks and sends #refresh, which sends #changed: #packageList.
and MCFileRepositoryInspector>>packageSearchAccept: [2]
also forks and sends #changed: #packageList
So there were three threads (including UI [3]) modifying object state,
which was compounded by #packageList method doing a lot of work. Issue
13007 worked around this by moving the work of #packageList out to
#packageListUpdate, and moved the #changed: sends from [1] & [2] also
out into #packageListUpdate (and wrapped in a #defer:).
_QUESTIONS_
Now I am considering doing the same for issue 13182
#versionSearchAccept:, and considering Pharo's goal is to replace all
Dependents/#changed: mechanisms with Announcements.
1. So I am wondering if that would be a better direction to address this?
Then I wonder about how Announcements interact with threads. I see that
SubscriptionRegistry>>deliver: #protect's the generation of
/interestedSubscriptions/, but the actual announcement delivery occurs
in the open, in AnnoucemenSubscription>>delvier:,
in the sender's thread.
So for an example...
action := [ :it | self inform: 'removing child'.].
announcer on: RemoveChild do: action.
/action/ might be executed from different concurrent threads.
Now you might say that the programmer should know enough about the
structure of their threaded code to wrap the action in as critical
section, but Announcements are a DECOUPLING mechanism, and maybe a THIRD
PARTY library subscribes to an Announcement that later someone else
wraps in a fork, and no-one notices possible race conditions created.
2. So, is there something reasonable we can do to make the
common/easier/safer single-threaded case the default, and make people
work a bit harder to shoot themselves in the foot with multiple threads?
We could introduce Announcer>>onOtherThread:do:
with the purpose of making use of inter-thread announcements more
explicit in a self documenting way.
Announcements accidentally passed between threads to Announcer>>on:do:
would generate an exception advising "use #onOtherThread:do: "
with the reasonable expectation that use of #onOtherThread:do:
would follow the pattern...
announcer onOtherThread: RemoveChild do: [ :it |
myMutex critical: [ self inform: 'removing child' ].
].
With this arrangement, a third party subscribed to someone else's
announcement, that is quietly changed to be wrapped by forked thread,
the third party's code now fails EARLY and OBVIOUS, rather than letting
subtle race conditions occur at random times.
_PROPOSED IMPLEMENTATION_
"------------------"
Announcer>>subscribe: anAnnouncementClass do: aValuable
^ registry add: (
AnnouncementSubscription new
announcer: self;
process: Processor activeProcess
announcementClass: anAnnouncementClass;
valuable: aValuable)
"------------------"
AnnouncementSubscription>>deliver: anAnnouncement
process ~= Processor activeProcess ifTrue: [
self error: [ 'Cross thread announcements should subscribe using
#onOtherThread:do: ' ].
^ (self handlesAnnouncement: anAnnouncement ) ifTrue: [
[action cull: anAnnouncement cull: announcer]
on: UnhandledError fork: [:ex | ex pass ]]
"------------------"
Announcer>>threadedSubscribe: anAnnouncementClass do: aValuable
^ registry add: (
ThreadedAnnouncementSubscription new
announcer: self;
process: Processor activeProcess
announcementClass: anAnnouncementClass;
valuable: aValuable)
"------------------"
where ThreadedAnnouncementSubscription essentially functions the same as
the existing AnnoucementSubscription .
