Fixes a bug that Ctrl-c not working on Salt CLI.

[kstreee]

What does this PR do?

Fixes a bug that Ctrl-c not working on Salt CLI because of this line. There is no way to terminate Salt CLI when the exception KeyboardInterrupt is caught by the line. It could be a problem for all kind of modules that call get_event directly.
In Salt Master, the above line is not a problem because of EventReturn will handle the exit signal on the background thread, but Salt CLI tools can't because they do not use the EventReturn. Also, it causes kind of memory leak that is caused by generating future object infinitely.
We know our solution, which uses graceful_exit flag, is not a beautiful way to solve this, but we think it is quite intuitive. We considered attaching EventReturn to Salt CLI processes, but we thought that the attaching solution is too much to solve this problem. If you have any other solution to resolve this issue, please let me know about it. I am open to discussion.
We could not check all call sites for the function get_event, thus if this commit is reasonable to fix this issue and is accepted, then other call sites should be reviewed.

F.Y.I. To minimize impacts of this commit, I set a default value of graceful_exit as True.

What issues does this PR fix or reference?

Previous Behavior

When users send a kill signal (for example, hitting Ctrl-c) while Salt CLI tools execute get_event, then the running Salt CLI process stalls and causes kind of memory leak.

New Behavior

Exits immediately, even Salt CLI received a kill signal while executing get_event.

Tests written?

No, I have no idea to make a test for Ctrl-c. If you have any examples or recommended way, I will gladly learn about it and make test cases.

[cachedout]

@s0undt3ch Since you've been doing so much signals/shutdown work lately I would love to have your opinion here.

[kstreee]

I just found another issue on this issue, so I want to suggest another way to fix the stalling and memory leaking bug.

First of all, check following toy example. the except KeyboardInterrupt statement is never called because of signal.

import signal
import time

def handler(a,b):
    print('signal received')
    exit()

def main():
    signal.signal(signal.SIGINT, handler)
    while True:
        try:
            time.sleep(1)
            print('working')
        except KeyboardInterrupt:
            print('keyboard interrupt received')

if __name__ == '__main__':
    main()

So the this line will never be called after signal handlers are set. Especially, Salt Master process never call the except KeyboardInterrupt statement. I found after this new issue, change my approach from modifying utils/event.py to modifying scripts.py. And the new approach, which patches scripts.py only, is less risk and more simple than the former one.
This stalling issue would be fixed by adding signal handlers in scripts.py, but still except KeyboardInterrupt statement in utils/event.py should be reviewed.

[cachedout]

I'm fine with this change on the CLI tools but could you please clarify what's broken with the current salt-master implementation of signal handling? I'm not following why that needs to be changed as well.

[kstreee]

@cachedout Thank you for your review, and sure, I will explain about the issue of signal and try exceptmore for you.

As I tested, the except KeyboardInterrupt statements are not executed after attaching signal handler functions using the function signal.signal(...), even users hit a Ctrl-c, the except KeyboardInterrupt statements will not be called. This Stackoverflow Q&A is talking about the control flow between signal and try except.

Because of the Python's control flow between signal and try except KeyboardInterrupt, the exception handling statement of this line in utils/event.py will never be called on salt-master, salt-minion, and other modules which attach signal handlers using signal.
To my knowledge, the tag salt/event/exit, which is only generated by the except KeyboardInterrrupt statement in utils/event.py, is only used by EventReturn class, and the EventReturn class is only used by the salt-master (and the function event_return_fork in daemons/flo/reactor.py, but I can't find that modules use the function event_return_fork). So, if the salt/event/exit handling logic is really necessary for salt-master, then the try except statement or signal attaching statement in salt-master should be changed, on the other hand, if the salt/event/exit tag is necessary, then the try except KeyboardInterrupt statement in utils/event.py could be removed without any risk.

[s0undt3ch]

This approach looks solid! Thanks @kstreee!

[cachedout]

Thanks for the explanation, @kstreee. I appreciate it. I agree that this looks like it's a solid approach. Let's go ahead and get this in.