-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor the control loop and handle sigterm #65
Conversation
@@ -184,10 +180,6 @@ def send(self, value): | |||
self._write_packet(data) | |||
return len(data) + self._packet_len.size | |||
|
|||
def recv(self, timeout=None): | |||
packet = self.reader_queue.get(block=True, timeout=timeout) | |||
return packet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got rid of the reader queue here and instead just invoke a callback letting the owner of the pipe handle packets how they wish (using a deque, notify the control pipe, etc)
|
||
def _signal_sighup(self, signum, frame): | ||
self.logger.info('Received SIGHUP, triggering a reload.') | ||
self.monitor.set_changed() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this old version was acquiring locks in the handler - now we just write to the control pipe
@@ -61,7 +61,7 @@ def is_alive(self): | |||
|
|||
def stop(self): | |||
if self.is_alive(): | |||
self.process.terminate() | |||
self.process.kill() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hupper now catches SIGTERM so instead we want to just SIGKILL in the test suite once we're done with the test
worker.wait(shutdown_interval) | ||
elif signal == ControlSignal.SIGCHLD: | ||
if not worker.is_alive(): | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently on windows SIGCHLD will not occur, but it's not a big deal because we have an open pipe with the child. When the child dies, the pipe is closed and we emit a WORKER_COMMAND with a None
packet and we're able to exit pretty quickly via the if cmd is None
check above.
bfcf284
to
1b68c43
Compare
102b4c0
to
4294fd2
Compare
4294fd2
to
cffa460
Compare
cffa460
to
c27df8f
Compare
8677f75
to
fbf8ab4
Compare
This way we make sure that when the other thread goes to read self.monitor.is_changed it is actually set, especially if for some reason sending the "signal" down the pipe causes the read to wakeup before the thread finishes processing.
This reverts commit fbf8ab4.
fixes #64
fixes #63