diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index 6dc856cbf..2e17ff50f 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -741,6 +741,14 @@ def __init__( self._task_id_guard = threading.Lock() self._can_protocol = CanProtocol.CAN_FD if fd else CanProtocol.CAN_20 + # poll() avoids select.select()'s ValueError for fds >= FD_SETSIZE. + # One poller per direction so a writable socket cannot wake up a + # blocking receive and concurrent send/recv share no state. + self._poll_in = select.poll() + self._poll_in.register(self.socket, select.POLLIN) + self._poll_out = select.poll() + self._poll_out.register(self.socket, select.POLLOUT) + # set the local_loopback parameter try: self.socket.setsockopt( @@ -815,9 +823,8 @@ def shutdown(self) -> None: def _recv_internal(self, timeout: float | None) -> tuple[Message | None, bool]: try: - # get all sockets that are ready (can be a list with a single value - # being self.socket or an empty list if self.socket is not ready) - ready_receive_sockets, _, _ = select.select([self.socket], [], [], timeout) + timeout_ms = -1 if timeout is None else max(0, int(timeout * 1000)) + ready_receive_sockets = self._poll_in.poll(timeout_ms) except OSError as error: # something bad happened (e.g. the interface went down) raise can.CanOperationError( @@ -859,7 +866,7 @@ def send(self, msg: Message, timeout: float | None = None) -> None: while time_left >= 0: # Wait for write availability - ready = select.select([], [self.socket], [], time_left)[1] + ready = self._poll_out.poll(int(time_left * 1000)) if not ready: # Timeout break diff --git a/doc/changelog.d/2053.fixed.rst b/doc/changelog.d/2053.fixed.rst new file mode 100644 index 000000000..fa24644da --- /dev/null +++ b/doc/changelog.d/2053.fixed.rst @@ -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()``. diff --git a/test/test_socketcan.py b/test/test_socketcan.py index 9d042f425..beeb1031a 100644 --- a/test/test_socketcan.py +++ b/test/test_socketcan.py @@ -5,6 +5,7 @@ """ import ctypes +import select import struct import sys import unittest @@ -26,6 +27,7 @@ build_bcm_transmit_header, build_bcm_tx_delete_header, build_bcm_update_header, + build_can_frame, ) from .config import IS_LINUX, IS_PYPY, TEST_INTERFACE_SOCKETCAN @@ -391,5 +393,52 @@ def test_pypy_socketcan_support(self): ) +@unittest.skipUnless(IS_LINUX, "socketcan is only available on Linux") +class SocketCANHighFdTest(unittest.TestCase): + """SocketcanBus must work with socket fds above FD_SETSIZE, see #2053.""" + + HIGH_FD = 2048 + + def setUp(self): + patcher_poll = patch("can.interfaces.socketcan.socketcan.select.poll") + patcher_create = patch("can.interfaces.socketcan.socketcan.create_socket") + patcher_bind = patch("can.interfaces.socketcan.socketcan.bind_socket") + self.mock_poller = patcher_poll.start().return_value + self.mock_socket = patcher_create.start().return_value + patcher_bind.start() + self.addCleanup(patcher_poll.stop) + self.addCleanup(patcher_create.stop) + self.addCleanup(patcher_bind.stop) + + self.mock_socket.fileno.return_value = self.HIGH_FD + self.bus = can.Bus(interface="socketcan", channel="can0") + self.addCleanup(self.bus.shutdown) + + def test_send_high_fd(self): + self.mock_poller.poll.return_value = [(self.HIGH_FD, select.POLLOUT)] + msg = can.Message(arbitration_id=0x123, data=[1, 2, 3, 4, 5, 6, 7, 8]) + frame = build_can_frame(msg) + self.mock_socket.send.return_value = len(frame) + + self.bus.send(msg) + + self.mock_socket.send.assert_called_once_with(frame) + + def test_recv_high_fd(self): + self.mock_poller.poll.return_value = [(self.HIGH_FD, select.POLLIN)] + expected_msg = can.Message( + arbitration_id=0x123, data=[1, 2, 3, 4, 5, 6, 7, 8], timestamp=1000.0 + ) + with patch( + "can.interfaces.socketcan.socketcan.capture_message" + ) as mock_capture: + mock_capture.return_value = expected_msg + msg = self.bus.recv(timeout=1.0) + + self.assertIsNotNone(msg) + self.assertEqual(msg.arbitration_id, 0x123) + mock_capture.assert_called_once_with(self.mock_socket, False) + + if __name__ == "__main__": unittest.main()