Skip to content

Avoid reentrancy in signal handler#92

Merged
dhood merged 1 commit intomasterfrom
avoid_reentrant_sigint_handler
Jun 24, 2018
Merged

Avoid reentrancy in signal handler#92
dhood merged 1 commit intomasterfrom
avoid_reentrant_sigint_handler

Conversation

@dhood
Copy link
Copy Markdown
Member

@dhood dhood commented Jun 23, 2018

@wjwwood what do you think of this to address #91? I got the idea from this blog post.

If it's alright I'll extend it to the other handlers.

@dhood dhood self-assigned this Jun 23, 2018
@dhood dhood added the in progress Actively being worked on (Kanban column) label Jun 23, 2018
prev_handler = signal.signal(signal.SIGINT, signal.SIG_IGN)
if prev_handler is signal.SIG_IGN:
# This function has been called re-entrantly.
return
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this expected to occur and be ok? If yes and then no, should it raise an exception?

Is this the case where a signal is received between 288 and 289?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am anticipating a race condition between 288 and 290, yeah. I haven't read anything to think that it's not possible.

I don't think that raising there is appropriate because the situation that caused #91 will then cause an exception, where I think that if we are already interrupting then there is no cause for concern. We print the message if we receive sigints later on but that's more for the information of the user from my understanding.

@dhood
Copy link
Copy Markdown
Member Author

dhood commented Jun 24, 2018

Merging this change that's been approved and will follow up with the other signal handlers

@dhood dhood merged commit 16ec361 into master Jun 24, 2018
@dhood dhood removed the in progress Actively being worked on (Kanban column) label Jun 24, 2018
@dhood dhood deleted the avoid_reentrant_sigint_handler branch June 24, 2018 05:22
@dhood
Copy link
Copy Markdown
Member Author

dhood commented Jun 24, 2018

This was responsible for instabilities in the nightlies last night because of a linter failure; addressed in ed75487

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants