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? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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