diff --git a/README.md b/README.md index cd88bd2..45b8778 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ import jsocket class Echo(jsocket.ThreadedServer): def __init__(self, **kwargs): super().__init__(**kwargs) - self.timeout = 2.0 + self.timeout = 2.0 # sets both accept and recv timeouts # Return a dict to send a response back to the client def _process_message(self, obj): @@ -70,7 +70,7 @@ import jsocket class Worker(jsocket.ServerFactoryThread): def __init__(self): super().__init__() - self.timeout = 2.0 + self.timeout = 2.0 # sets recv timeout for this worker def _process_message(self, obj): if isinstance(obj, dict) and 'message' in obj: @@ -89,7 +89,9 @@ API Highlights - `connect()` returns True on success - `send_obj(dict)` sends a JSON object - `read_obj()` blocks until a full message is received; raises `socket.timeout` or `RuntimeError("socket connection broken")` - - `timeout` property controls socket timeouts + - `timeout` sets both accept and recv timeouts + - `accept_timeout` controls the server's accept timeout + - `recv_timeout` controls the connection read timeout - ThreadedServer: - Subclass and implement `_process_message(self, obj) -> Optional[dict]` diff --git a/features/steps/steps.py b/features/steps/steps.py index bed5321..92d188f 100644 --- a/features/steps/steps.py +++ b/features/steps/steps.py @@ -23,7 +23,8 @@ class MyServer(jsocket.ThreadedServer): def __init__(self, **kwargs): super().__init__(**kwargs) - self.timeout = 2.0 + if not any(key in kwargs for key in ("timeout", "accept_timeout", "recv_timeout")): + self.timeout = 2.0 def _process_message(self, obj): # Echo payloads that include 'echo' for verification @@ -41,6 +42,33 @@ def start_server(context): _, context.server_port = context.jsonserver.socket.getsockname() +@given(r"I start the server with accept timeout (\d+(?:\.\d+)?) seconds") +def start_server_with_accept_timeout(context, seconds): + # Bind to an ephemeral port to avoid port conflicts + context.jsonserver = MyServer( + address='127.0.0.1', + port=0, + accept_timeout=float(seconds), + ) + context.jsonserver.start() + # Discover the chosen port + _, context.server_port = context.jsonserver.socket.getsockname() + + +@given(r"I start the server with accept timeout (\d+(?:\.\d+)?) seconds and recv timeout (\d+(?:\.\d+)?) seconds") +def start_server_with_accept_and_recv_timeout(context, accept_seconds, recv_seconds): + # Bind to an ephemeral port to avoid port conflicts + context.jsonserver = MyServer( + address='127.0.0.1', + port=0, + accept_timeout=float(accept_seconds), + recv_timeout=float(recv_seconds), + ) + context.jsonserver.start() + # Discover the chosen port + _, context.server_port = context.jsonserver.socket.getsockname() + + @given(r"I connect the client") def connect_client(context): port = getattr(context, 'server_port', None) @@ -56,6 +84,11 @@ def client_sends_object(context, obj): context.jsonclient.send_obj(json.loads(obj)) +@when(r"I wait (\d+(?:\.\d+)?) seconds") +def wait_seconds(context, seconds): + time.sleep(float(seconds)) + + @when(r"the server sends the object (\{.*\})") def server_sends_object(context, obj): # Wait briefly until the server has an accepted connection @@ -83,6 +116,7 @@ def within_seconds_server_connected(context, seconds): @given(r"I stop the server") +@when(r"I stop the server") def stop_server(context): server = getattr(context, 'jsonserver', None) if server is not None: diff --git a/features/timeout_behavior.feature b/features/timeout_behavior.feature new file mode 100644 index 0000000..ce43e2b --- /dev/null +++ b/features/timeout_behavior.feature @@ -0,0 +1,14 @@ +#@PydevCodeAnalysisIgnore +Feature: Timeout Behavior + + Scenario: Stop server despite long accept timeout + Given I start the server with accept timeout 10.0 seconds + When I stop the server + Then the server is stopped + + Scenario: Separate accept and recv timeouts + Given I start the server with accept timeout 10.0 seconds and recv timeout 0.2 seconds + And I connect the client + When I wait 0.3 seconds + And the client sends the object {"echo": "after-idle"} + Then the client sees a message {"echo": "after-idle"} diff --git a/jsocket/_version.py b/jsocket/_version.py index f911ef0..a8e4cfd 100644 --- a/jsocket/_version.py +++ b/jsocket/_version.py @@ -1,3 +1,3 @@ """Package version.""" -__version__ = "2.0.0" +__version__ = "2.0.1" diff --git a/jsocket/jsocket_base.py b/jsocket/jsocket_base.py index fb653f9..0048fa4 100644 --- a/jsocket/jsocket_base.py +++ b/jsocket/jsocket_base.py @@ -50,17 +50,32 @@ def _socket_fileno(sock): class JsonSocket: """Lightweight JSON-over-TCP socket wrapper with length-prefixed framing.""" - def __init__(self, address='127.0.0.1', port=5489, timeout=2.0, max_message_size=DEFAULT_MAX_MESSAGE_SIZE): - self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - self.conn = self.socket + def __init__( + self, + address='127.0.0.1', + port=5489, + timeout=2.0, + max_message_size=DEFAULT_MAX_MESSAGE_SIZE, + accept_timeout=None, + recv_timeout=None, + create_socket=True, + ): + self.socket = None + self.conn = None self._timeout = timeout + self._accept_timeout = timeout if accept_timeout is None else accept_timeout + self._recv_timeout = timeout if recv_timeout is None else recv_timeout self._address = address self._port = port self._max_message_size = None self.max_message_size = max_message_size self._last_client_addr = None - # Ensure the primary socket respects timeout for accept/connect operations - self.socket.settimeout(self._timeout) + self._is_server = False + if create_socket: + self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self.conn = self.socket + # Primary socket timeout (accept for servers; connect/read for clients). + self.socket.settimeout(self._accept_timeout) def send_obj(self, obj): """Send a JSON-serializable object over the connection.""" @@ -172,12 +187,48 @@ def _close_connection(self): def _get_timeout(self): """Get the current socket timeout in seconds.""" - return self._timeout + return getattr(self, "_timeout", None) def _set_timeout(self, timeout): """Set the socket timeout in seconds and apply to the main socket.""" self._timeout = timeout - self.socket.settimeout(timeout) + self._accept_timeout = timeout + self._recv_timeout = timeout + sock = getattr(self, "socket", None) + if sock is not None: + sock.settimeout(self._accept_timeout) + conn = getattr(self, "conn", None) + if conn is not None: + if getattr(self, "_is_server", False): + if conn is not sock: + conn.settimeout(self._recv_timeout) + else: + conn.settimeout(self._recv_timeout) + + def _get_accept_timeout(self): + """Get the listening/accept timeout in seconds.""" + return getattr(self, "_accept_timeout", getattr(self, "_timeout", None)) + + def _set_accept_timeout(self, timeout): + """Set the listening/accept timeout in seconds.""" + self._accept_timeout = timeout + sock = getattr(self, "socket", None) + if sock is not None: + sock.settimeout(timeout) + + def _get_recv_timeout(self): + """Get the connection recv timeout in seconds.""" + return getattr(self, "_recv_timeout", getattr(self, "_timeout", None)) + + def _set_recv_timeout(self, timeout): + """Set the connection recv timeout in seconds.""" + self._recv_timeout = timeout + conn = getattr(self, "conn", None) + sock = getattr(self, "socket", None) + if conn is not None: + if getattr(self, "_is_server", False) and conn is sock: + return + conn.settimeout(timeout) def _get_address(self): """Return the configured bind address.""" @@ -209,7 +260,9 @@ def _set_max_message_size(self, size): raise ValueError("max_message_size must be positive") self._max_message_size = size - timeout = property(_get_timeout, _set_timeout, doc='Get/set the socket timeout') + timeout = property(_get_timeout, _set_timeout, doc='Get/set accept/recv timeout together') + accept_timeout = property(_get_accept_timeout, _set_accept_timeout, doc='Get/set accept timeout') + recv_timeout = property(_get_recv_timeout, _set_recv_timeout, doc='Get/set recv timeout') address = property(_get_address, _set_address, doc='read only property socket address') port = property(_get_port, _set_port, doc='read only property socket port') max_message_size = property(_get_max_message_size, _set_max_message_size, doc='Get/set max message size in bytes') @@ -218,8 +271,15 @@ def _set_max_message_size(self, size): class JsonServer(JsonSocket): """Server socket that accepts one connection at a time.""" - def __init__(self, address='127.0.0.1', port=5489): - super().__init__(address, port) + def __init__(self, address='127.0.0.1', port=5489, timeout=2.0, accept_timeout=None, recv_timeout=None): + super().__init__( + address, + port, + timeout=timeout, + accept_timeout=accept_timeout, + recv_timeout=recv_timeout, + ) + self._is_server = True self._bind() def _close_connection(self): @@ -253,7 +313,7 @@ def accept_connection(self): self._listen() self.conn, addr = self._accept() self._last_client_addr = addr - self.conn.settimeout(self.timeout) + self.conn.settimeout(self.recv_timeout) logger.debug( "connection accepted, conn socket (%s,%d,%s)", addr[0], addr[1], str(self.conn.gettimeout()) ) @@ -274,8 +334,10 @@ def _is_connected(self): class JsonClient(JsonSocket): """Client socket for connecting to a JsonServer and exchanging JSON messages.""" - def __init__(self, address='127.0.0.1', port=5489): - super().__init__(address, port) + def __init__(self, address='127.0.0.1', port=5489, timeout=2.0, recv_timeout=None): + super().__init__(address, port, timeout=timeout, recv_timeout=recv_timeout) + if self.socket is not None: + self.socket.settimeout(self._timeout) def connect(self): """Attempt to connect to the server up to 10 times with backoff.""" @@ -294,5 +356,7 @@ def connect(self): time.sleep(3) continue logger.info("...Socket Connected") + # Switch to recv_timeout after successful connection + self.socket.settimeout(self._recv_timeout) return True return False diff --git a/jsocket/tserver.py b/jsocket/tserver.py index 7aed190..c2a753c 100644 --- a/jsocket/tserver.py +++ b/jsocket/tserver.py @@ -22,6 +22,7 @@ """ import threading import socket +import select import time import logging import abc @@ -61,6 +62,91 @@ def __init__(self, **kwargs): self._stats_lock = threading.Lock() self._client_started_at = None self._client_id = None + self._wakeup_r = None + self._wakeup_w = None + self._init_wakeup() + + def _init_wakeup(self): + """Create a wakeup socketpair to interrupt blocking accept.""" + try: + self._wakeup_r, self._wakeup_w = socket.socketpair() + self._wakeup_r.setblocking(False) + self._wakeup_w.setblocking(False) + return + except (AttributeError, OSError): + pass + # Fallback: create a loopback TCP pair + try: + listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + writer = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + writer.connect(listener.getsockname()) + reader, _ = listener.accept() + listener.close() + reader.setblocking(False) + writer.setblocking(False) + self._wakeup_r, self._wakeup_w = reader, writer + except OSError: + self._wakeup_r, self._wakeup_w = None, None + + def _signal_wakeup(self): + """Wake the accept loop if it is blocked.""" + sock = getattr(self, "_wakeup_w", None) + if sock is None: + return + try: + sock.send(b"\x00") + except OSError: + pass + + def _drain_wakeup(self): + sock = getattr(self, "_wakeup_r", None) + if sock is None: + return + try: + while True: + if not sock.recv(1024): + break + except (BlockingIOError, OSError): + pass + + def _close_wakeup(self): + for sock in (getattr(self, "_wakeup_r", None), getattr(self, "_wakeup_w", None)): + if sock is None: + continue + try: + sock.close() + except OSError: + pass + self._wakeup_r = None + self._wakeup_w = None + + def _wait_for_accept(self) -> bool: + """Block until a client can be accepted or a wakeup is signaled.""" + sock = getattr(self, "socket", None) + wake = getattr(self, "_wakeup_r", None) + if sock is None or wake is None: + return True + try: + if hasattr(self, "_listen"): + self._listen() + except OSError as e: + logger.debug("listen error on %s:%s: %s", self.address, self.port, e) + return False + while getattr(self, "_is_alive", False): + try: + readable, _, _ = select.select([sock, wake], [], [], None) + except (OSError, ValueError) as e: + logger.debug("select error on %s:%s: %s", self.address, self.port, e) + return False + if wake in readable: + self._drain_wakeup() + return False + if sock in readable: + return True + return False @abc.abstractmethod def _process_message(self, obj) -> Optional[dict]: @@ -102,6 +188,8 @@ def get_client_stats(self) -> dict: def _accept_client(self) -> bool: """Accept an incoming connection; return True when a client connects.""" + if not self._wait_for_accept(): + return False try: self.accept_connection() except socket.timeout as e: @@ -155,6 +243,7 @@ def run(self): self.close() except OSError: pass + self._close_wakeup() def start(self): """ Starts the threaded server. @@ -174,6 +263,7 @@ def stop(self): """ self._is_alive = False self._clear_client_stats() + self._signal_wakeup() logger.debug("Threaded Server stopped on %s:%s", self.address, self.port) @@ -181,10 +271,15 @@ class ServerFactoryThread(threading.Thread, jsocket_base.JsonSocket, metaclass=a """Per-connection worker thread used by ServerFactory.""" def __init__(self, **kwargs): - threading.Thread.__init__(self, **kwargs) + create_socket = kwargs.pop("create_socket", False) + thread_kwargs = {} + for key in ("group", "target", "name", "args", "kwargs", "daemon"): + if key in kwargs: + thread_kwargs[key] = kwargs.pop(key) + threading.Thread.__init__(self, **thread_kwargs) self.socket = None self.conn = None - jsocket_base.JsonSocket.__init__(self, **kwargs) + jsocket_base.JsonSocket.__init__(self, create_socket=create_socket, **kwargs) self._is_alive = False self._client_started_at = None self._client_id = None @@ -195,8 +290,20 @@ def swap_socket(self, new_sock): @param new_sock socket to replace the existing default jsocket.JsonSocket object @retval None """ + existing_socket = getattr(self, "socket", None) + if existing_socket is not None and existing_socket is not new_sock: + try: + self._close_socket() + except OSError: + pass self.socket = new_sock self.conn = self.socket + try: + timeout = getattr(self, "_recv_timeout", getattr(self, "_timeout", None)) + if timeout is not None: + self.socket.settimeout(timeout) + except OSError: + pass try: addr = new_sock.getpeername() except OSError: @@ -260,7 +367,17 @@ def force_stop(self): class ServerFactory(ThreadedServer): """Accepts clients and spawns a ServerFactoryThread per connection.""" def __init__(self, server_thread, **kwargs): - ThreadedServer.__init__(self, address=kwargs['address'], port=kwargs['port']) + init_kwargs = { + "address": kwargs["address"], + "port": kwargs["port"], + } + if "timeout" in kwargs: + init_kwargs["timeout"] = kwargs["timeout"] + if "accept_timeout" in kwargs: + init_kwargs["accept_timeout"] = kwargs["accept_timeout"] + if "recv_timeout" in kwargs: + init_kwargs["recv_timeout"] = kwargs["recv_timeout"] + ThreadedServer.__init__(self, **init_kwargs) if not issubclass(server_thread, ServerFactoryThread): raise TypeError("serverThread not of type", ServerFactoryThread) self._thread_type = server_thread @@ -269,6 +386,7 @@ def __init__(self, server_thread, **kwargs): self._thread_args = kwargs self._thread_args.pop('address', None) self._thread_args.pop('port', None) + self._thread_args.pop('accept_timeout', None) def _process_message(self, obj) -> Optional[dict]: """ServerFactory does not process messages itself.""" @@ -283,6 +401,8 @@ def run(self): tmp = self._thread_type(**self._thread_args) self._purge_threads() while not self.connected and self._is_alive: + if not self._wait_for_accept(): + continue try: self.accept_connection() except socket.timeout as e: @@ -336,6 +456,7 @@ def _purge_threads(self): def stop(self): # Stop accepting and stop all workers self._is_alive = False + self._signal_wakeup() try: self.stop_all() except Exception: # pylint: disable=broad-exception-caught diff --git a/pyproject.toml b/pyproject.toml index 0a1867d..d8156c1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ dynamic = ["version"] description = "Python JSON Server & Client" readme = "README.md" requires-python = ">=3.8" -license = {file = "LICENSE"} +license = {text = "Apache-2.0"} authors = [{name = "Christopher Piekarski", email = "chris@cpiekarski.com"}] maintainers = [{name = "Christopher Piekarski", email = "chris@cpiekarski.com"}] keywords = ["json", "socket", "server", "client"] diff --git a/tests/test_additional_coverage.py b/tests/test_additional_coverage.py index eba1296..e523799 100644 --- a/tests/test_additional_coverage.py +++ b/tests/test_additional_coverage.py @@ -9,6 +9,7 @@ import pytest import jsocket +import jsocket.tserver as tserver def test_client_connect_failure_returns_false(monkeypatch): @@ -226,8 +227,339 @@ def flappy_accept(): raise RuntimeError("accept stopped") monkeypatch.setattr(server, "accept_connection", flappy_accept) + monkeypatch.setattr(server, "_wait_for_accept", lambda: True) t = threading.Thread(target=server.run, daemon=True) t.start() t.join(timeout=1.0) assert calls["n"] >= 1 + + +def test_wakeup_init_fallback_success(monkeypatch): + """_init_wakeup should fall back to TCP pair when socketpair is unavailable.""" + + class WakeServer(tserver.ThreadedServer): + def __init__(self): + threading.Thread.__init__(self) + self._wakeup_r = None + self._wakeup_w = None + + def _process_message(self, obj): # pragma: no cover - not used + return None + + class FakeReader: + def setblocking(self, flag): # pylint: disable=unused-argument + return None + + class FakeWriter: + def __init__(self): + self.connected = False + + def connect(self, addr): # pylint: disable=unused-argument + self.connected = True + + def setblocking(self, flag): # pylint: disable=unused-argument + return None + + class FakeListener: + def __init__(self, reader): + self.reader = reader + + def setsockopt(self, *args, **kwargs): # pylint: disable=unused-argument + return None + + def bind(self, addr): # pylint: disable=unused-argument + return None + + def listen(self, backlog): # pylint: disable=unused-argument + return None + + def getsockname(self): + return ("127.0.0.1", 12345) + + def accept(self): + return self.reader, ("127.0.0.1", 12345) + + def close(self): + return None + + reader = FakeReader() + writer = FakeWriter() + listener = FakeListener(reader) + sockets = [listener, writer] + + def fake_socket(*args, **kwargs): # pylint: disable=unused-argument + return sockets.pop(0) + + monkeypatch.setattr(socket, "socketpair", lambda: (_ for _ in ()).throw(AttributeError())) + monkeypatch.setattr(socket, "socket", fake_socket, raising=True) + + srv = WakeServer() + srv._init_wakeup() + + assert srv._wakeup_r is reader + assert srv._wakeup_w is writer + + +def test_wakeup_init_fallback_failure(monkeypatch): + """_init_wakeup should leave wakeup sockets unset when fallback fails.""" + + class WakeServer(tserver.ThreadedServer): + def __init__(self): + threading.Thread.__init__(self) + self._wakeup_r = None + self._wakeup_w = None + + def _process_message(self, obj): # pragma: no cover - not used + return None + + monkeypatch.setattr(socket, "socketpair", lambda: (_ for _ in ()).throw(AttributeError())) + monkeypatch.setattr(socket, "socket", lambda *args, **kwargs: (_ for _ in ()).throw(OSError())) + + srv = WakeServer() + srv._init_wakeup() + assert srv._wakeup_r is None + assert srv._wakeup_w is None + + +def test_wakeup_signal_and_drain_cover_errors(monkeypatch): + """_signal_wakeup and _drain_wakeup should ignore socket errors.""" + + class WakeServer(tserver.ThreadedServer): + def __init__(self): + threading.Thread.__init__(self) + self._wakeup_r = None + self._wakeup_w = None + + def _process_message(self, obj): # pragma: no cover - not used + return None + + class BadSender: + def send(self, data): # pylint: disable=unused-argument + raise OSError("boom") + + class BadReader: + def recv(self, size): # pylint: disable=unused-argument + raise OSError("boom") + + srv = WakeServer() + srv._wakeup_w = BadSender() + srv._signal_wakeup() + + srv._wakeup_r = None + srv._drain_wakeup() + + srv._wakeup_r = BadReader() + srv._drain_wakeup() + + +def test_wakeup_close_handles_errors(): + """_close_wakeup should ignore close errors.""" + + class WakeServer(tserver.ThreadedServer): + def __init__(self): + threading.Thread.__init__(self) + self._wakeup_r = None + self._wakeup_w = None + + def _process_message(self, obj): # pragma: no cover - not used + return None + + class BadClose: + def close(self): + raise OSError("boom") + + srv = WakeServer() + srv._wakeup_r = BadClose() + srv._wakeup_w = BadClose() + srv._close_wakeup() + assert srv._wakeup_r is None + assert srv._wakeup_w is None + + +def test_wait_for_accept_paths(monkeypatch): + """_wait_for_accept should handle listen/select/wakeup paths.""" + + class WaitServer(tserver.ThreadedServer): + def __init__(self): + threading.Thread.__init__(self) + self._is_alive = True + self.socket = object() + self._wakeup_r = object() + self._address = "127.0.0.1" + self._port = 0 + + def _process_message(self, obj): # pragma: no cover - not used + return None + + def _listen(self): + return None + + srv = WaitServer() + + def listen_fail(): + raise OSError("boom") + + srv._listen = listen_fail + assert srv._wait_for_accept() is False + + srv._listen = lambda: None + monkeypatch.setattr(tserver.select, "select", lambda *_: (_ for _ in ()).throw(ValueError("boom"))) + assert srv._wait_for_accept() is False + + drained = {"n": 0} + + def drain(): + drained["n"] += 1 + + srv._drain_wakeup = drain + monkeypatch.setattr(tserver.select, "select", lambda *_: ([srv._wakeup_r], [], [])) + assert srv._wait_for_accept() is False + assert drained["n"] == 1 + + monkeypatch.setattr(tserver.select, "select", lambda *_: ([srv.socket], [], [])) + assert srv._wait_for_accept() is True + + srv._is_alive = False + assert srv._wait_for_accept() is False + + +def test_accept_client_timeout_branch(): + """_accept_client should return False on socket.timeout.""" + + class TimeoutServer(tserver.ThreadedServer): + def __init__(self): + threading.Thread.__init__(self) + self._is_alive = True + self._stats_lock = threading.Lock() + self._client_started_at = None + self._client_id = None + self._address = "127.0.0.1" + self._port = 0 + + def _process_message(self, obj): # pragma: no cover - not used + return None + + def _wait_for_accept(self): + return True + + def accept_connection(self): + raise socket.timeout("t") + + srv = TimeoutServer() + assert srv._accept_client() is False + + +def test_serverfactorythread_kwargs_and_swap_error_branches(): + """ServerFactoryThread should accept thread kwargs and ignore swap errors.""" + + class Worker(tserver.ServerFactoryThread): + def _process_message(self, obj): # pragma: no cover - not used + return None + + worker = Worker(name="w1", daemon=True) + assert worker.name == "w1" + assert worker.daemon is True + + class BadSocket: + def settimeout(self, timeout): # pylint: disable=unused-argument + raise OSError("boom") + + def getpeername(self): + return ("127.0.0.1", 12345) + + worker.socket = object() + + def close_raises(): + raise OSError("boom") + + worker._close_socket = close_raises + worker.swap_socket(BadSocket()) + + +def test_serverfactorythread_run_with_none_response(): + """ServerFactoryThread.run should handle None responses.""" + + class NoneRespWorker(tserver.ServerFactoryThread): + # pylint: disable=non-parent-init-called + def __init__(self): + threading.Thread.__init__(self) + self._is_alive = True + self.closed = False + self.socket = object() + + def read_obj(self): + self._is_alive = False + return {"x": 1} + + def _process_message(self, obj): # pylint: disable=unused-argument + return None + + def _close_connection(self): + self.closed = True + + def _close_socket(self): + return None + + worker = NoneRespWorker() + worker.run() + assert worker.closed is True + + +def test_serverfactory_accept_timeout_and_close_branches(monkeypatch): + """ServerFactory.run should handle accept timeouts and close accepted conns on stop.""" + + def fake_init(self, address, port, timeout=2.0, accept_timeout=None, recv_timeout=None): + self.address = address + self.port = port + self._address = address + self._port = port + self.socket = object() + self.conn = self.socket + self._is_alive = True + self._stats_lock = threading.Lock() + self._client_started_at = None + self._client_id = None + self._threads = [] + self._threads_lock = threading.Lock() + + monkeypatch.setattr(tserver.ThreadedServer, "__init__", fake_init, raising=True) + + class Worker(tserver.ServerFactoryThread): + def _process_message(self, obj): # pragma: no cover - not used + return None + + factory = tserver.ServerFactory(Worker, address="127.0.0.1", port=0) + monkeypatch.setattr(factory, "_wait_for_accept", lambda: True) + monkeypatch.setattr(factory, "_purge_threads", lambda: None) + monkeypatch.setattr(factory, "_wait_to_exit", lambda: None) + monkeypatch.setattr(factory, "close", lambda: None) + + called = {"timeout": 0} + + def accept_timeout(): + called["timeout"] += 1 + factory._is_alive = False + raise socket.timeout("t") + + monkeypatch.setattr(factory, "accept_connection", accept_timeout) + factory.run() + assert called["timeout"] == 1 + + class CloseRaises: + def shutdown(self, how): # pylint: disable=unused-argument + raise OSError("boom") + + def close(self): + raise OSError("boom") + + def fileno(self): # pragma: no cover - safety for connected() + return 5 + + def accept_then_stop(): + factory.conn = CloseRaises() + factory._is_alive = False + + monkeypatch.setattr(factory, "accept_connection", accept_then_stop) + factory._is_alive = True + factory.run() diff --git a/tests/test_integration_failures.py b/tests/test_integration_failures.py index ce22454..cd06a80 100644 --- a/tests/test_integration_failures.py +++ b/tests/test_integration_failures.py @@ -48,6 +48,42 @@ def _process_message(self, obj): # pylint: disable=unused-argument return None +class SlowResponseServer(jsocket.ThreadedServer): + """Server that delays its response to trigger client timeouts.""" + + def __init__(self, delay, **kwargs): + super().__init__(**kwargs) + self.timeout = 0.5 + self._delay = delay + + def _process_message(self, obj): + time.sleep(self._delay) + if isinstance(obj, dict) and "echo" in obj: + return obj + return None + + +class FirstSlowResponseServer(jsocket.ThreadedServer): + """Server that delays only the first response.""" + + def __init__(self, first_delay, **kwargs): + super().__init__(**kwargs) + self.timeout = 0.5 + self._first_delay = first_delay + self._count = 0 + self._count_lock = threading.Lock() + + def _process_message(self, obj): + with self._count_lock: + self._count += 1 + count = self._count + if count == 1: + time.sleep(self._first_delay) + if isinstance(obj, dict) and "echo" in obj: + return obj + return None + + def _send_partial_payload(address, port, payload, partial_len): sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) try: @@ -207,6 +243,79 @@ def test_threadedserver_client_read_times_out_without_response(): server.join(timeout=3) +@pytest.mark.integration +@pytest.mark.timeout(10) +def test_client_timeout_then_reads_late_response(): + """Client can timeout on the first read and still receive a delayed response.""" + try: + server = SlowResponseServer(delay=0.3, address="127.0.0.1", port=0) + except PermissionError as e: + pytest.skip(f"Socket creation blocked: {e}") + + client = None + _, port = server.socket.getsockname() + server.start() + + try: + client = jsocket.JsonClient(address="127.0.0.1", port=port) + client.timeout = 0.1 + assert client.connect() is True + payload = {"echo": "late-response"} + client.send_obj(payload) + with pytest.raises(socket.timeout): + client.read_obj() + + client.timeout = 2.0 + assert client.read_obj() == payload + finally: + if client is not None: + try: + client.close() + except OSError: + pass + server.stop() + server.join(timeout=3) + + +@pytest.mark.integration +@pytest.mark.timeout(10) +def test_client_timeout_then_send_another_message(): + """Client can send another message after a read timeout.""" + try: + server = FirstSlowResponseServer(first_delay=0.3, address="127.0.0.1", port=0) + except PermissionError as e: + pytest.skip(f"Socket creation blocked: {e}") + + client = None + _, port = server.socket.getsockname() + server.start() + + try: + client = jsocket.JsonClient(address="127.0.0.1", port=port) + client.timeout = 0.1 + assert client.connect() is True + + first = {"echo": "first"} + client.send_obj(first) + with pytest.raises(socket.timeout): + client.read_obj() + + second = {"echo": "second"} + client.send_obj(second) + + client.timeout = 2.0 + assert client.read_obj() == first + assert client.read_obj() == second + finally: + if client is not None: + try: + client.close() + except OSError: + pass + server.stop() + server.join(timeout=3) + + @pytest.mark.integration @pytest.mark.timeout(10) def test_client_raises_framing_error_on_checksum_mismatch(): diff --git a/tests/test_unit_coverage.py b/tests/test_unit_coverage.py index d6ace44..505e5f2 100644 --- a/tests/test_unit_coverage.py +++ b/tests/test_unit_coverage.py @@ -44,6 +44,22 @@ def send(self, data): return len(data) +class TimeoutSocket: # pylint: disable=missing-function-docstring + """Socket stub that records timeout settings.""" + + def __init__(self): + self.timeouts = [] + + def settimeout(self, timeout): + self.timeouts.append(timeout) + + def getpeername(self): + return ("127.0.0.1", 12345) + + def fileno(self): # pragma: no cover - safety for debug paths + return 1 + + def _make_base_socket(): """Build a JsonSocket stub with a fake connection.""" sock = jsocket_base.JsonSocket.__new__(jsocket_base.JsonSocket) @@ -70,6 +86,104 @@ def test_send_obj_rejects_oversize(): sock.send_obj({"x": "too-big"}) +def test_jsonclient_init_skips_settimeout_when_socket_missing(monkeypatch): + """JsonClient should tolerate a JsonSocket init that leaves socket unset.""" + + def fake_init(self, address='127.0.0.1', port=0, timeout=2.0, recv_timeout=None, **kwargs): + self.socket = None + self.conn = None + self._timeout = timeout + self._recv_timeout = timeout if recv_timeout is None else recv_timeout + + monkeypatch.setattr(jsocket_base.JsonSocket, "__init__", fake_init, raising=True) + + client = jsocket_base.JsonClient(address="127.0.0.1", port=0) + assert client.socket is None + + +def test_timeout_sets_accept_and_recv_timeouts(): + """timeout should update accept_timeout and recv_timeout.""" + sock = jsocket_base.JsonSocket(create_socket=False) + sock.socket = TimeoutSocket() + sock.conn = TimeoutSocket() + sock._is_server = False + + sock.timeout = 1.5 + + assert sock.timeout == 1.5 + assert sock.accept_timeout == 1.5 + assert sock.recv_timeout == 1.5 + assert sock.socket.timeouts == [1.5] + assert sock.conn.timeouts == [1.5] + + +def test_timeout_sets_recv_on_server_connection(): + """timeout should update recv timeout for server connections.""" + sock = jsocket_base.JsonSocket(create_socket=False) + sock.socket = TimeoutSocket() + sock.conn = TimeoutSocket() + sock._is_server = True + + sock.timeout = 2.5 + + assert sock.socket.timeouts == [2.5] + assert sock.conn.timeouts == [2.5] + + +def test_accept_timeout_updates_listening_socket_only(): + """accept_timeout should only update the listening socket.""" + sock = jsocket_base.JsonSocket(create_socket=False) + sock.socket = TimeoutSocket() + sock.conn = TimeoutSocket() + sock._is_server = True + + sock.accept_timeout = 3.0 + + assert sock.socket.timeouts == [3.0] + assert sock.conn.timeouts == [] + assert sock.recv_timeout == 2.0 + + +def test_accept_timeout_no_socket_noop(): + """accept_timeout should be a no-op when no socket exists.""" + sock = jsocket_base.JsonSocket(create_socket=False) + sock.accept_timeout = 6.0 + assert sock.accept_timeout == 6.0 + + +def test_recv_timeout_updates_connection_socket_only(): + """recv_timeout should only update the connection socket.""" + sock = jsocket_base.JsonSocket(create_socket=False) + sock.socket = TimeoutSocket() + sock.conn = TimeoutSocket() + sock._is_server = True + + sock.recv_timeout = 4.0 + + assert sock.socket.timeouts == [] + assert sock.conn.timeouts == [4.0] + assert sock.accept_timeout == 2.0 + + +def test_recv_timeout_skips_listen_socket_for_server(): + """recv_timeout should not update when conn is the listening socket.""" + sock = jsocket_base.JsonSocket(create_socket=False) + sock.socket = TimeoutSocket() + sock.conn = sock.socket + sock._is_server = True + + sock.recv_timeout = 3.0 + + assert sock.socket.timeouts == [] + + +def test_recv_timeout_no_conn_noop(): + """recv_timeout should no-op when no connection exists.""" + sock = jsocket_base.JsonSocket(create_socket=False) + sock.recv_timeout = 7.0 + assert sock.recv_timeout == 7.0 + + def test_read_header_invalid_magic_closes(): """_read_header should raise when magic bytes are invalid.""" sock = _make_base_socket() @@ -532,6 +646,53 @@ def fake_init(self, address, port): # pylint: disable=unused-argument assert factory._process_message({"x": 1}) is None +def test_serverfactory_routes_accept_and_recv_timeouts(monkeypatch): + """ServerFactory should apply accept_timeout to the factory and recv_timeout to workers.""" + captured = {} + + def fake_init(self, address, port, timeout=2.0, accept_timeout=None, recv_timeout=None): + """Stub ThreadedServer initializer for isolation.""" + captured["timeout"] = timeout + captured["accept_timeout"] = accept_timeout + captured["recv_timeout"] = recv_timeout + self.address = address + self.port = port + self.conn = None + self.socket = None + self._is_alive = False + self._stats_lock = threading.Lock() + self._client_started_at = None + self._client_id = None + self._accept_timeout = accept_timeout + self._recv_timeout = recv_timeout + + monkeypatch.setattr(tserver.ThreadedServer, "__init__", fake_init, raising=True) + + class Worker(tserver.ServerFactoryThread): + def _process_message(self, obj): # pragma: no cover - not used + return None + + factory = tserver.ServerFactory( + Worker, + address="127.0.0.1", + port=0, + timeout=5.0, + accept_timeout=10.0, + recv_timeout=2.0, + ) + + assert captured["timeout"] == 5.0 + assert captured["accept_timeout"] == 10.0 + assert captured["recv_timeout"] == 2.0 + assert "accept_timeout" not in factory._thread_args + assert factory._thread_args["recv_timeout"] == 2.0 + + worker = Worker(**factory._thread_args) + sock = TimeoutSocket() + worker.swap_socket(sock) + assert sock.timeouts == [2.0] + + def test_serverfactory_run_closes_accepted_conn_when_stopping(): """ServerFactory should close accepted connections when stopping.""" factory = _StoppingFactory()