adinapoli / threads-supervisor

Simple, IO-based Haskell library for Erlang-inspired thread supervisors
MIT License
29 stars 4 forks source link

Exception safety #5

Open adinapoli opened 9 years ago

adinapoli commented 9 years ago

A valid comment on Reddit:

For example, imaging the next sequence: 
  - child thread throws exception; 
  - supervised sends DeadLetter; 
  - on other thread shutdownSupervisor kills the child (note: the child is already dead, so nothing happens); 
  - now monitoring thread (in handleEvents) receives the DeadLetter and respawns the child;  
  - shutdownSupervisor kills the monitoring thread. 
Now you have an orphan child.
srijs commented 8 years ago

Could it be that the better ordering in this case is to shut down the supervisor first, and then the children? (Might need to mask shutdownSupervisor in addition to that).

Alternatively, the supervisor could hold a "shutdown flag" in STM, and on shutdownSupervisor that would be set together with reading the children reference (would require that to be a TVar instead of IORef though). handleEvents would read the flag first before proceeding.

adinapoli commented 8 years ago

@srijs now that we have epochs, do you think that comment has still value? In theory what should happen in the described scenario is that the letter would be considered stale and ignored.

Perhaps we should have a test case replicating that scenario, making sure that's indeed the case.

srijs commented 8 years ago

I'm not convinced the introduction of epochs alone solves this problem yet.

I imagine the easiest way to achieve thread safety for actions on supervisors would be to linearise all actions. This is partly what we have already with the handleEvents loop, just that it currently handles only dead letter events.

If we extended the supervisors ingress message queue to also carry shutdown commands (and have shutdownSupervisor simply queue such a message), that would give us the guarantee that access to the supervisor's IORef'd children is serialised and we don't need to care about thread-safety in this particular aspect of the system.

(Incidentally this is exactly how Erlang does it too :P)

srijs commented 8 years ago

@adinapoli threw together a rough sketch of what I mean here: #26

adinapoli commented 8 years ago

@srijs I like this approach! Incidentally, it seems we are slowly converging towards the same pattern (see my PR on epochs): having the other functions writing richer messages inside the queue, and having everything processed by handleEvents :wink: