Mic92 / python-mpd2

Python library which provides a client interface for the Music Player Daemon.
GNU Lesser General Public License v3.0
352 stars 119 forks source link

Close transport on asyncio client disconnect #99

Closed mgrachten closed 5 years ago

mgrachten commented 6 years ago

The current disconnect method of asyncio.MPDClient does not close the writing transport. A warning for this is issued when running the python3 interpreter with -Wdefault. As a result, the MPD server keeps alive the connection to the client unnecessarily. If this happens repeatedly, the dangling connections may cause the MPD server to reach its maximal connection limit.

mgrachten commented 6 years ago

A minimal working example to show the issue is:

import asyncio
from mpd.asyncio import MPDClient

async def main():
    client = MPDClient()
    try:
        await client.connect('localhost', 6600)
    except Exception as e:
        print("Connection failed:", e)
        return

    # Uncommenting the following line avoids the warning
    # client._MPDClient__wfile.close()

    client.disconnect()

if __name__ == '__main__':
    loop = asyncio.get_event_loop()
    loop.run_until_complete(main())
    loop.close()

Running the above script using python3 -Wdefault will issue a resource warning:

/usr/lib/python3.5/asyncio/selector_events.py:574: ResourceWarning: unclosed transport <_SelectorSocketTransport fd=6> warnings.warn("unclosed transport %r" % self, ResourceWarning)

Uncommenting the line client._MPDClient__wfile.close() (as a proof of concept) avoids the warning.

When the transport is not closed, the connection to the MPD server is maintained until the script exits.

Mic92 commented 6 years ago

The ancient python version that breaks on travis was removed in our CI.

varac commented 6 years ago

What's the current state ? This PR is pending for quite a while.

mgrachten commented 6 years ago

As far as I know this PR is still pending review by @supermihi, but it's really a tiny PR, so maybe somebody else can review.