Stub and server lifecycle fixes #4269
Conversation
For a termination action to properly mark an _End as having stopped it must clear the _End's _cycle attribute. To be able to do that the termination action must hold a reference to the _End instance. Migrating the _termination_action behavior that creates termination actions into the scope of the _End instance is the best way to afford the needed instance reference.
Context management is implemented. Stub deletion now cancels all RPCs immediately.
| def _stop_now(self): | ||
| with self._lock: | ||
| if self._end_link is not None: | ||
| if self._stop_events is None: |
There was a problem hiding this comment.
This appears to be the only place where the nullability of _stop_events comes into play for branching purposes, and appears to be replaceable with a cast to bool . Is there any other reason to have _stop_events be nullable and have the extra creation logic in _schedule_stop? (this is more of a just-in-case than a I-have-a-strong-opinion thing)
There was a problem hiding this comment.
The non-None-ness of _stop_events indicates that the server is "stopping" (was asked to stop and was giving a positive grace). So when _end_link is non-None and _stop_events is None, that indicates that the server is fully up. Also the identity of _stop_events is used by passing it to the thread that waits during the grace period.
There was a problem hiding this comment.
I see that, I meant only that it looks like all that was said could just as well have been done without relying on None-ness. I don't recall ever querying you before for your opinion on None as a matter of style, s'all.
There was a problem hiding this comment.
Except for the identity thing (not directly anyway). What's the situation being guarded against? Multiple stops and really bad thread scheduling?
There was a problem hiding this comment.
None is a very effective sentinel, and I do like using it in complex-state-objects to indicate "this field is not valid when the object is in this category of state".
There was a problem hiding this comment.
Exactly. Consider:
(1) Server is running.
(2) Server is told to stop with grace; thread is kicked off.
(3) Server is told to stop with no grace. Grace thread is unblocked on its event, but for some reason doesn't get scheduled on a CPU. Server stops.
(4) Server is restarted.
(5) Server is told to stop with long grace.
(6) Thread kicked off in (2) is finally scheduled and runs on a CPU.
Without the identity comparison, that thread would acquire the lock, see that the server is in a shutdown grace period, and complete the shutdown. With the identity comparison, the thread is able to determine that although the server is in a shutdown grace period, it is not in the same shutdown grace period as the one the thread was scheduled to conclude, and so it correctly does nothing.
Stub and server lifecycle fixes
Fixes at least #3820, #529 and possibly a few other problems too.