fix(socketcan): support socket file descriptors over 1023#2057
Conversation
select.select() is limited by glibc's FD_SETSIZE (1024) and raises ValueError for higher fds even when the OS limit allows them. Switch SocketcanBus._recv_internal() and send() to select.poll(), which has no such limit. Fixes hardbyte#2053. Signed-off-by: SAY-5 <say.apm35@gmail.com>
Signed-off-by: SAY-5 <say.apm35@gmail.com>
SAY-5
force-pushed
the
fix/socketcan-high-fd
branch
from
5d2dca1 to
43dc1a5
Compare
| while time_left >= 0: | ||
| # Wait for write availability | ||
| ready = select.select([], [self.socket], [], time_left)[1] | ||
| ready = poller.poll(max(0, int(time_left * 1000))) |
There was a problem hiding this comment.
What's the max call for? time_left is alwas positive here.
There was a problem hiding this comment.
Dropped in 977fad4, the loop condition already guarantees time_left >= 0.
| # being self.socket or an empty list if self.socket is not ready) | ||
| ready_receive_sockets, _, _ = select.select([self.socket], [], [], timeout) | ||
| # poll() avoids select.select()'s ValueError for fds >= FD_SETSIZE | ||
| poller = select.poll() |
There was a problem hiding this comment.
You create new poll objects on every read and write. That can't be good for performance.
Create one poll object inside __init__() and register both POLLIN and POLLOUT. Then here you call (fd, event) = self._poller.poll() and check whether the event contains POLLIN.
There was a problem hiding this comment.
Done in 977fad4. I went with one poller per direction, both created in init: with a single object registered for POLLIN|POLLOUT, recv would wake immediately whenever the socket is writable (which is almost always) and turn a blocking recv into a spin. Two pollers keep the blocking semantics and also avoid send/recv threads mutating shared registration state.
| @@ -0,0 +1,91 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
You don't need to create a new file to test this. Add a test to the existing socketcan tests.
There was a problem hiding this comment.
Moved into test_socketcan.py as SocketCANHighFdTest in 977fad4 and deleted the extra file.
| @@ -0,0 +1 @@ | |||
| ``SocketcanBus`` now uses ``select.poll()`` instead of ``select.select()`` so that socket file descriptors above ``FD_SETSIZE`` (1024 on glibc) no longer raise ``ValueError: filedescriptor out of range in select()``. | |||
…st into test_socketcan Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
2 participants
Summary of Changes
SocketcanBus._recv_internal()andsend()fromselect.select()toselect.poll().test/test_socketcan_high_fd.py(Linux-only) covering send/recv with a mocked socket whose fd is 2048.Related Issues / Pull Requests
SocketcanBusdoes not support socket file descriptors over 1023 #2053Type of Change
Checklist
tox).Additional Notes
select.select()is limited by glibc'sFD_SETSIZE(1024) and raisesValueError: filedescriptor out of range in select()for higher fds even when the OS limit allows them.select.poll()has no such limit and returns equivalent readiness data. The change is internal toSocketcanBus; the public send/recv signatures and exception semantics are unchanged.