Session/socket never closed, warnings shown

[HansWeltar]

When you run python with -Wdefault (show warnings) and do some work with the msal lib, the following warning will be displayed on stdout: sys:1: ResourceWarning: unclosed <ssl.SSLSocket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6, laddr=('10.200.33.175', 36376), raddr=('20.190.160.69', 443)>

It seems http_client and it's underlying socket are never closed by the msal lib. This causes python to print a warning on stdout (when running as a unittest or with -Wdefault) By default http_client is a requests.Session and these keep the socket open until the Session is explicitely closed. However we found no place in the code where 'close' is called on http_client.

Steps to reproduce the behavior:

import msal
import warnings
# APPID="...."
# SECRET="...."
# TENANT="...."
warnings.filterwarnings('always')  # enable warnings, same as python3 -Wdefault
app=msal.application.ConfidentialClientApplication(APPID, client_credential=SECRET, authority="https://login.microsoftonline.com/" + TENANT)
app.acquire_token_for_client(scopes=["https://graph.microsoft.com/.default"])
del(app)  # -> triggers the unclosed socket warning

Or, to use one of the samples: python3 -Wdefault confidential_client_secret_sample.py config.json For us this shows:

$ python3 -Wdefault confidential_client_secret_sample.py config.json 
confidential_client_secret_sample.py:42: ResourceWarning: unclosed file <_io.TextIOWrapper name='config.json' mode='r' encoding='UTF-8'>
  config = json.load(open(sys.argv[1]))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
Graph API call result: {
  "@odata.context": "https://graph.microsoft.com/v1.0/$metadata#users",
  "value": [
    {
      ....
    }
  ]
}
sys:1: ResourceWarning: unclosed <ssl.SSLSocket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6, laddr=('10.200.33.175', 33236), raddr=('40.126.31.6', 443)>
ResourceWarning: Enable tracemalloc to get the object allocation traceback

Note: The ResourceWarnings might not always show at the end of the program (depends on the garbage collector). To reliably reproduce this issue, use the snippet above with the explicit del(app) Also ignore the first resource warning, that's an unclosed file in the sample, and has nothing to do with the error in the lib)

config.json:

{
    "authority": "https://login.microsoftonline.com/*****",
    "client_id": "*****",
    "scope": ["https://graph.microsoft.com/.default"],
    "secret": "*****",
    "endpoint": "https://graph.microsoft.com/v1.0/users"
}

Expected behavior No warnings and a possibility to close http_client. (like eg: app.close())

What you see instead

sys:1: ResourceWarning: unclosed <ssl.SSLSocket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6, laddr=('10.200.33.175', 56906), raddr=('20.190.160.134', 443)>
ResourceWarning: Enable tracemalloc to get the object allocation traceback

The MSAL Python version you are using Tested with msal 1.11.0, 1.1.0 and git master branch

[rayluo]

Hi Hans, thanks for the report, and your attempt to fix it. We will take a look into your PR and especially the remaining issue that you also pointed out in that PR.

This issue should not have high impact on real world usage, though, because it is expected that MSAL ConfidentialClientApplication or PublicClientApplication instance should ideally be a long-lived object until your entire application exits. That way, it would be easier for MSAL to reuse its internal resources, and your app would have better overall performance.

[HansWeltar]

We're using msal for usersyncing every half an hour (time is configurable though). So we'd like to close sockets between runs.

Hi Hans, thanks for the report, and your attempt to fix it. We will take a look into your PR and especially the remaining issue that you also pointed out in that PR.

This issue should not have high impact on real world usage, though, because it is expected that MSAL ConfidentialClientApplication or PublicClientApplication instance should ideally be a long-lived object until your entire application exits. That way, it would be easier for MSAL to reuse its internal resources, and your app would have better overall performance.