Conversation
ncw
left a comment
There was a problem hiding this comment.
This looks like a nice change - thank you
See inline for comments
| closed bool | ||
| running sync.WaitGroup | ||
| done chan struct{} | ||
| interrupt int32 // atomic; non-zero means KeyboardInterrupt pending |
There was a problem hiding this comment.
Using atomic.Int32 here saves mistakes
There was a problem hiding this comment.
Good point! I forgot we are still on 1.8. Maybe we should upgrade the minimum requirements. What do you think?
| defer ctx.Close() | ||
|
|
||
| sigCh := make(chan os.Signal, 1) | ||
| signal.Notify(sigCh, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) |
There was a problem hiding this comment.
This won't work on windows I think.
We probably only need os.Interrupt anyway, INT and TERM are not generated by the keyboard.
Perhaps see what python3 does with those signals?
There was a problem hiding this comment.
"INT and TERM are not generated by the keyboard." You sure? I thought that INT was called by pressing CTRL+C.
There was a problem hiding this comment.
You are right of course. However os.Interrrupt is the same as sigint on Unix machines, so it should cover everything we need
Adding context interrupting based on atomics, for breaking code runner.