Possible waste of new pooled-connections on application shutdown

[ankostis]

On python-shutdown of an app using this lib I get these debug logs from these protocol lines:

2017-06-27 10:52:53,489:DEBUG:exchangelib.autodiscover:Domain outlook.com: Closing sessions
2017-06-27 10:52:53,490:DEBUG:exchangelib.protocol:Server outlook.com: Closing sessions
2017-06-27 10:52:53,490:DEBUG:exchangelib.protocol:Closing socket ('1.2.3.4', 24179)
2017-06-27 10:52:53,491:DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): outlook.com
2017-06-27 10:52:53,491:DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): outlook.com
2017-06-27 10:52:53,492:DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): outlook.com
2017-06-27 10:52:53,492:DEBUG:exchangelib.protocol:Server outlook.com: Closing sessions
2017-06-27 10:52:53,492:DEBUG:exchangelib.protocol:Server outlook.office365.com: Closing sessions
2017-06-27 10:52:53,492:DEBUG:exchangelib.protocol:Closing socket ('1.2.3.4', 24183)
2017-06-27 10:52:53,493:DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): outlook.office365.com
2017-06-27 10:52:53,493:DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): outlook.office365.com
2017-06-27 10:52:53,494:DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): outlook.office365.com

• Could it be that when retrieving the connections from the pool to close them, actually new ones are generated?
• Wouldn't it be enough to invoke urllib3.connectionpool.HttpConnectionPool.close() , like that?

    def close_socket(self, url):
        # Close underlying socket. This ensures we don't leave stray sockets around after program exit.
        adapter = self.get_adapter(url)
        pool = adapter.get_connection(url)
+         pool.close()
-         for _ in range(pool.pool.qsize()):
-             conn = pool._get_conn()
-             if conn.sock:
-                 log.debug('Closing socket %s', text_type(conn.sock.getsockname()))
-                 conn.sock.shutdown(socket.SHUT_RDWR)
-                 conn.sock.close()

[ecederstrand]

I agree the behaviour you logged is not intended.

There should have been a comment there describing the seemingly unnecessary work. I seem to remember adding this because pool.close() would not actually close the raw socket connection, and I'd get warnings about open resources on process exit. Possibly this was a workaround for requests/requests-ntlm#51

Maybe this workaround is no longer needed in newer versions of requests?

[ankostis]

Have no idea how to test it.
I can just tell you that with my edit above, I got no ResourceWarning.
[edit:] Specifically, these are the logs I'm getting with just pool.close():

2017-06-27 11:42:27,438:DEBUG:exchangelib.autodiscover:Domain outlook.com: Closing sessions
2017-06-27 11:42:27,438:DEBUG:exchangelib.protocol:Server outlook.com: Closing sessions
2017-06-27 11:42:27,440:DEBUG:exchangelib.protocol:Server outlook.com: Closing sessions
2017-06-27 11:42:27,440:DEBUG:exchangelib.protocol:Server outlook.office365.com: Closing sessions

[utapyngo]

Please release a new version to PyPI. I keep getting an error without this fix.

[ecederstrand]

Yup, will do as soon as I get time to run the test suite locally. Hopefully tonight.

[ecederstrand]

I just released v1.9.5