altdesktop / python-dbus-next

🚌 The next great DBus library for Python with asyncio support
https://python-dbus-next.readthedocs.io/en/latest/
MIT License
187 stars 59 forks source link

Fix socket creation on macos. SOCK_NONBLOCK does not exists. #63

Closed kapetan closed 3 years ago

kapetan commented 3 years ago

socket.SOCK_NONBLOCK does not exist on macOS, so creating a dbus instance with an unix path fails. This fix uses same socket creation as with tcp. This works on macOS 10.15.6 with dbus 1.12.20, but I don't know if it has any consequences on other platforms.

Running the tests locally with:

DBUS_SESSION_BUS_ADDRESS="unix:path=${DBUS_LAUNCHD_SESSION_BUS_SOCKET}" python3 -m pytest

Where DBUS_LAUNCHD_SESSION_BUS_SOCKET is something like /private/tmp/com.apple.launchd/unix_domain_listener.

Results in two failed tests:

test_signals_with_changing_owners
OSError: [Errno 57] Socket is not connected
test_tcp_connection_with_forwarding
AssertionError: assert 'abstract' in {'path': '/private/tmp/com.apple.launchd/unix_domain_listener'}

If I use path instead of abstract as dictionary key in test_tcp_connection_with_forwarding the test passes.

acrisci commented 3 years ago

I can't guarantee MacOS support because there's no way I can test on this platform, but I would like to support platforms without nonblocking sockets. This is a feature only available in Linux >= 2.6.27. So you should add a feature check using platform.system. None of the code should require nonblocking sockets, but it is compatible with them.

michallowasrzechonek-silvair commented 3 years ago

Any platform that supports python3 and asyncio has nonblocking sockets :|

acrisci commented 3 years ago

Oh yeah you're right. I misread the python docs. It's just a different way of setting the nonblocking socket in Darwin. I think you have to set O_NONBLOCK with fcntl() after socket creation in that case.

kapetan commented 3 years ago

What's the correct approach here? I can add something like.

flag = fcntl.fcntl(self._sock.fileno(), fcntl.F_GETFL)
fcntl.fcntl(self._sock.fileno(), fcntl.F_SETFL, flag | os.O_NONBLOCK)

It can be hidden behind a feature flag.

acrisci commented 3 years ago

I think that's probably already done in the call to sock.setblocking(False) so this is probably fine.

acrisci commented 3 years ago

:+1: