msrest locks asyncio when getting credentials

[alexrecuenco]

I was noticing using the botbuilder-python infrastructure how the event loop just gets stuck.

I used pyinstrument, and I tracked it down to this line.

https://github.com/Azure/msrest-for-python/blob/af41991f17445e4cb26d39c247f091ddfd7027b9/msrest/pipeline/async_requests.py#L99

Is this a misuse by botbuilder-python of the msrest library? Is this user error? or is this just a bug on the implementation of msrest? I would appreciate some feedback on how to proceed

I leave the async profiling trace here

Format of this text would be

• time (in seconds)
• function name
• file path
• repeat

botbuilder/dialogs/prompts/oauth_prompt.py:179
1.936
OAuthPrompt._recognize_token
botbuilder/dialogs/prompts/oauth_prompt.py:395
1.933
get_user_token
botbuilder/dialogs/_user_token_access.py:19
1.933
_UserTokenClientImpl.get_user_token
botframework/connector/auth/_user_token_client_impl.py:34
1.933
UserTokenOperations.get_token
botframework/connector/token_api/aio/operations_async/_user_token_operations_async.py:36
1.931
ServiceClientAsync.async_send
msrest/async_client.py:103
1.931
AsyncPipeline.run
msrest/pipeline/async_abc.py:155
1.931
_SansIOAsyncHTTPPolicyRunner.send
msrest/pipeline/async_abc.py:76
1.931
AsyncRequestsCredentialsPolicy.send
msrest/pipeline/async_requests.py:96
1.677
MicrosoftAppCredentials.signed_session
botframework/connector/auth/app_credentials.py:81
1.677
MicrosoftAppCredentials.get_access_token
botframework/connector/auth/microsoft_app_credentials.py:37
1.286
ConfidentialClientApplication.acquire_token_for_client
msal/application.py:2350
1.286
ConfidentialClientApplication._acquire_token_silent_with_error
msal/application.py:1451
1.286
ConfidentialClientApplication._acquire_token_silent_from_cache_and_possibly_refresh_it
msal/application.py:1513
1.286
ConfidentialClientApplication._acquire_token_for_client
msal/application.py:2376
1.286
_ClientWithCcsRoutingInfo.obtain_token_for_client
msal/oauth2cli/oauth2.py:745
1.286
_ClientWithCcsRoutingInfo._obtain_token
msal/oauth2cli/oidc.py:166
1.285
_ClientWithCcsRoutingInfo._obtain_token
msal/oauth2cli/oauth2.py:770
1.284
_ClientWithCcsRoutingInfo._obtain_token
msal/oauth2cli/oauth2.py:185
1.284
wrapper
msal/individual_cache.py:255
1.284
wrapper
msal/individual_cache.py:255
1.284
Session.post
requests/sessions.py:626
1.284
Session.request
requests/sessions.py:500
1.275
Session.send
requests/sessions.py:673
1.272
HTTPAdapter.send
requests/adapters.py:613
1.268
HTTPSConnectionPool.urlopen
urllib3/connectionpool.py:592
1.267
HTTPSConnectionPool._make_request
urllib3/connectionpool.py:377
1.267
HTTPSConnection.getresponse
urllib3/connection.py:485
1.266
HTTPSConnection.getresponse
http/client.py:1386
1.266
HTTPResponse.begin
http/client.py:324
1.263
HTTPResponse._read_status
http/client.py:291
1.263
SocketIO.readinto
socket.py:706
1.263
SSLSocket.recv_into
ssl.py:1236
1.263
SSLSocket.read
ssl.py:1094
1.262
_SSLSocket.read

[alexrecuenco]

[alexrecuenco]

The really ugly workaround would be await asyncio.to_thread(...), but I have no idea if in general self._creds.signed_session(session) is thread-safe. I would hope so

[alexrecuenco]

As a workaround I am monkey-patching like this

def monkey_patch_msrest_async_requests():
    # Added to_thread to prevent asyncio locking https://github.com/Azure/msrest-for-python/issues/264
    from msrest.pipeline.async_requests import (
        _LOGGER,
        AsyncRequestsCredentialsPolicy,
        ClientRequestError,
        TokenExpiredError,
        oauth2,
        raise_with_traceback,
        requests,
    )

    async def send(self, request, **kwargs):
        session = request.context.session
        try:
            await asyncio.to_thread(self._creds.signed_session, session)
        except TypeError:  # Credentials does not support session injection
            _LOGGER.warning(
                "Your credentials class does not support session injection. Performance will not be at the maximum."
            )
            request.context.session = session = await asyncio.to_thread(self._creds.signed_session)

        try:
            try:
                return await self.next.send(request, **kwargs)
            except (
                oauth2.rfc6749.errors.InvalidGrantError,
                oauth2.rfc6749.errors.TokenExpiredError,
            ):
                error = "Token expired or is invalid. Attempting to refresh."
                _LOGGER.warning(error)

            try:
                try:
                    await asyncio.to_thread(self._creds.refresh_session, session)
                except TypeError:  # Credentials does not support session injection
                    _LOGGER.warning(
                        "Your credentials class does not support session injection."
                        "Performance will not be at the maximum."
                    )
                    request.context.session = session = await asyncio.to_thread(
                        self._creds.refresh_session
                    )

                return await self.next.send(request, **kwargs)
            except (
                oauth2.rfc6749.errors.InvalidGrantError,
                oauth2.rfc6749.errors.TokenExpiredError,
            ) as err:
                msg = "Token expired or is invalid."
                raise_with_traceback(TokenExpiredError, msg, err)

        except (requests.RequestException, oauth2.rfc6749.errors.OAuth2Error) as err:
            msg = "Error occurred in request."
            raise_with_traceback(ClientRequestError, msg, err)

    AsyncRequestsCredentialsPolicy.send = send

Then calling this function monkey_patch_msrest_async_requests() early on startup

[alexrecuenco]

I am sure this workaround will break at some point with some concurrency issue, but for now it at least not blocking the entire application for a second at a time