Skip to content

Allow interactive flow to be aborted by CTRL+C even when running on Windows #404

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

Merged
merged 4 commits into from
Sep 9, 2021

Conversation

rayluo
Copy link
Collaborator

@rayluo rayluo commented Sep 9, 2021

This PR fix #393 which only happens on Windows.

@rayluo rayluo merged commit 9c08533 into dev Sep 9, 2021
@rayluo rayluo deleted the ctrlc branch September 9, 2021 17:09
@jiasli
Copy link
Contributor

jiasli commented Sep 10, 2021

Tested in Azure CLI with the latest MSAL dev branch. Upon pressing Ctrl+C, MSAL reported a call stack:

Exception in thread Thread-1:
Traceback (most recent call last):
  File "C:\Users\user1\AppData\Local\Programs\Python\Python39\lib\threading.py", line 973, in _bootstrap_inner
    self.run()
  File "C:\Users\user1\AppData\Local\Programs\Python\Python39\lib\threading.py", line 910, in run
    self._target(*self._args, **self._kwargs)
  File "d:\cli-msal\microsoft-authentication-library-for-python\msal\oauth2cli\authcode.py", line 264, in _get_auth_response
    self._server.handle_request()
  File "C:\Users\user1\AppData\Local\Programs\Python\Python39\lib\socketserver.py", line 291, in handle_request
    selector.register(self, selectors.EVENT_READ)
  File "C:\Users\user1\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 300, in register
    key = super().register(fileobj, events, data)
  File "C:\Users\user1\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 239, in register
    key = SelectorKey(fileobj, self._fileobj_lookup(fileobj), events, data)
  File "C:\Users\user1\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 226, in _fileobj_lookup
    return _fileobj_to_fd(fileobj)
  File "C:\Users\user1\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 42, in _fileobj_to_fd
    raise ValueError("Invalid file descriptor: {}".format(fd))
ValueError: Invalid file descriptor: -1

Weird thing is that if I move

out of the while True loop, the call stack is gone.

@jiasli
Copy link
Contributor

jiasli commented Sep 10, 2021

Reproduced with an easier sample:

import threading
import time


from http.server import HTTPServer, SimpleHTTPRequestHandler


class MyClass:
    def __init__(self):
        self.server = HTTPServer(('127.0.0.1', 8400), SimpleHTTPRequestHandler)

    def _get_auth_response(self, result):
        while True:
            print(">>> Calling handle_request")
            self.server.handle_request()
            print("<<< handle_request exits")

    def get_auth_response(self):
        result = {}
        t = threading.Thread(target=self._get_auth_response, args=(result,))
        t.daemon = True
        t.start()
        while True:
            time.sleep(1)
            if not t.is_alive():
                break
        return result or None

    def close(self):
        """Either call this eventually; or use the entire class as context manager"""
        self.server.server_close()

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        self.close()


try:
    with MyClass() as receiver:
        print(receiver.get_auth_response())
except KeyboardInterrupt:
    print("cancelled")
    # Give the daemon thread some time to exit
    time.sleep(1)

Output:

> python main.py
>>> Calling handle_request
<<< handle_request exits
cancelled
>>> Calling handle_request
Exception in thread Thread-1:
Traceback (most recent call last):
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\threading.py", line 973, in _bootstrap_inner
    self.run()
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\threading.py", line 910, in run
    self._target(*self._args, **self._kwargs)
  File "D:\cli\testproj\main.py", line 15, in _get_auth_response
    self.server.handle_request()
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\socketserver.py", line 291, in handle_request
    selector.register(self, selectors.EVENT_READ)
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 300, in register
    key = super().register(fileobj, events, data)
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 239, in register
    key = SelectorKey(fileobj, self._fileobj_lookup(fileobj), events, data)
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 226, in _fileobj_lookup
    return _fileobj_to_fd(fileobj)
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 42, in _fileobj_to_fd
    raise ValueError("Invalid file descriptor: {}".format(fd))
ValueError: Invalid file descriptor: -1

Notice >>> Calling handle_request appears twice.

Explanation:

With the shutdown of the main thread, the first invocation of self.server.handle_request() exits
-> Due to the outer while True, self.server.handle_request() is called again
-> Since the server has been closed by context manager MyClass.__exit__, self.server.handle_request() raises error.

@jiasli
Copy link
Contributor

jiasli commented Sep 10, 2021

Weirdly enough, if I make odd number of GET requests before pressing Ctrl+C, the above script never fails. If I make even number of GET requests before pressing Ctrl+C, the above script always fails.

Edit: I think this depends on Chrome holds the socket (#405 (comment)).

@rayluo
Copy link
Collaborator Author

rayluo commented Sep 10, 2021

Reproduced with an easier sample:

...

Output:

> python main.py
>>> Calling handle_request
<<< handle_request exits
cancelled
>>> Calling handle_request
Exception in thread Thread-1:
Traceback (most recent call last):
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\threading.py", line 973, in _bootstrap_inner
    self.run()
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\threading.py", line 910, in run
    self._target(*self._args, **self._kwargs)
  File "D:\cli\testproj\main.py", line 15, in _get_auth_response
    self.server.handle_request()
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\socketserver.py", line 291, in handle_request
    selector.register(self, selectors.EVENT_READ)
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 300, in register
    key = super().register(fileobj, events, data)
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 239, in register
    key = SelectorKey(fileobj, self._fileobj_lookup(fileobj), events, data)
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 226, in _fileobj_lookup
    return _fileobj_to_fd(fileobj)
  File "C:\Users\jiasli\AppData\Local\Programs\Python\Python39\lib\selectors.py", line 42, in _fileobj_to_fd
    raise ValueError("Invalid file descriptor: {}".format(fd))
ValueError: Invalid file descriptor: -1

Notice >>> Calling handle_request appears twice.

Explanation:

With the shutdown of the main thread, the first invocation of self.server.handle_request() exits
-> Due to the outer while True, self.server.handle_request() is called again
-> Since the server has been closed by context manager MyClass.__exit__, self.server.handle_request() raises error.

Thank you, @jiasli , for your investigation! Inspired by your experiment, I ended up using a simpler yet more accurate improvement. You can test it again using the latest MSAL dev branch.

@rayluo rayluo mentioned this pull request Sep 30, 2021
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.

acquire_token_interactive can't be stopped by Ctrl+C
2 participants