diff --git a/keyext.api.client/src/test/java/edu/kit/keyext/client/RPCLayerTest.java b/keyext.api.client/src/test/java/edu/kit/keyext/client/RPCLayerTest.java index 5226f3446d3..76fb5f48985 100644 --- a/keyext.api.client/src/test/java/edu/kit/keyext/client/RPCLayerTest.java +++ b/keyext.api.client/src/test/java/edu/kit/keyext/client/RPCLayerTest.java @@ -43,15 +43,4 @@ void testIncoming() throws IOException { String second = listener.readMessage(); Assertions.assertEquals(response, second); } - - - - @Test - void testLockingAndReleasing() throws IOException, InterruptedException { - var response = JsonRPC.addHeader(JsonRPC.createResponse("0", 2)); - var layer = new RPCLayer(new StringReader(response), new StringWriter()); - layer.start(); // starts the thread. - var result = layer.callSync("calc", 1, 1); - System.out.println(result); - } } diff --git a/keyext.client.python/keyapi/rpc.py b/keyext.client.python/keyapi/rpc.py index 5f3d75092b2..71dff15c39d 100644 --- a/keyext.client.python/keyapi/rpc.py +++ b/keyext.client.python/keyapi/rpc.py @@ -122,7 +122,7 @@ def object_decoder(obj): class LspEndpoint(threading.Thread): - def __init__(self, json_rpc_endpoint: JsonRpcEndpoint, method_callbacks=None, notify_callbacks=None, timeout=2000): + def __init__(self, json_rpc_endpoint: JsonRpcEndpoint, method_callbacks=None, notify_callbacks=None, timeout=None): super().__init__() self.json_rpc_endpoint: JsonRpcEndpoint = json_rpc_endpoint self.notify_callbacks: Dict = notify_callbacks or {} @@ -144,6 +144,20 @@ def stop(self): self.shutdown_flag = True def run(self): + try: + self._run_loop() + finally: + # Stop and wake any callers still waiting so they fail fast instead + # of blocking forever once the connection is gone. + self.shutdown_flag = True + self._wake_pending() + + def _wake_pending(self): + for cond in list(self.event_dict.values()): + with cond: + cond.notify_all() + + def _run_loop(self): rpc_id = None while not self.shutdown_flag: try: @@ -201,16 +215,20 @@ def call_method(self, method_name, args): cond = threading.Condition() self.event_dict[current_id] = cond - cond.acquire() - self.send_message(method_name, args, current_id) - if self.shutdown_flag: - return None - - if not cond.wait(timeout=self._timeout): - raise TimeoutError() - cond.release() - - self.event_dict.pop(current_id) + with cond: + self.send_message(method_name, args, current_id) + # `timeout` is in seconds; None waits indefinitely. KeY operations + # can run for a long time, so a premature timeout is worse than none. + if not self.shutdown_flag: + cond.wait(timeout=self._timeout) + + self.event_dict.pop(current_id, None) + if current_id not in self.response_dict: + # Woken without a result: the wait timed out, or the reader thread + # stopped because the server closed the connection. + if self.shutdown_flag: + raise ConnectionError("server closed the connection before responding") + raise TimeoutError("no response within %s seconds" % self._timeout) result, error = self.response_dict.pop(current_id) if error: raise ResponseError(error.get("code"), error.get("message"), error.get("data")) diff --git a/keyext.client.python/tests/__init__.py b/keyext.client.python/tests/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/keyext.client.python/tests/test_lsp_endpoint.py b/keyext.client.python/tests/test_lsp_endpoint.py new file mode 100644 index 00000000000..fff84619b66 --- /dev/null +++ b/keyext.client.python/tests/test_lsp_endpoint.py @@ -0,0 +1,81 @@ +# This file is part of KeY - https://key-project.org +# KeY is licensed under the GNU General Public License Version 2 +# SPDX-License-Identifier: GPL-2.0-only +"""Stability tests for keyapi.rpc.LspEndpoint (timeout semantics, disconnect). + +Run with:: + + python3 -m unittest discover -s tests -t . + +from the ``keyext.client.python`` directory. +""" +import threading +import time +import unittest + +from keyapi.rpc import LspEndpoint + + +class _SilentEndpoint: + """Never responds and never closes — used to exercise the timeout path.""" + + def send_request(self, message): + pass + + def recv_response(self): + time.sleep(3600) + return None + + +class _DisconnectingEndpoint: + """Simulates the server closing the connection shortly after a request.""" + + def send_request(self, message): + pass + + def recv_response(self): + time.sleep(0.1) + return None # connection closed + + +class LspEndpointTimeoutTest(unittest.TestCase): + def test_default_timeout_is_none(self): + # #13: the default must be an unambiguous "wait indefinitely", not the + # old 2000 that looked like milliseconds but meant ~33 minutes (seconds). + self.assertIsNone(LspEndpoint(_SilentEndpoint())._timeout) + + def test_finite_timeout_is_seconds_and_raises(self): + endpoint = LspEndpoint(_SilentEndpoint(), timeout=0.2) + endpoint.daemon = True + endpoint.start() + start = time.monotonic() + with self.assertRaises(TimeoutError): + endpoint.call_method("ping", []) + # 0.2 is interpreted as seconds, so the call returns promptly. + self.assertLess(time.monotonic() - start, 5) + + def test_server_disconnect_wakes_pending_call(self): + # With no timeout a disconnect must still wake the caller (ConnectionError) + # instead of hanging forever. + endpoint = LspEndpoint(_DisconnectingEndpoint()) + endpoint.daemon = True + endpoint.start() + + outcome = {} + + def call(): + try: + endpoint.call_method("ping", []) + except Exception as exc: # noqa: BLE001 - recorded for assertion + outcome["exc"] = exc + + t = threading.Thread(target=call) + t.daemon = True # so an unfixed (hanging) endpoint can't block the suite + t.start() + t.join(timeout=5) + self.assertFalse(t.is_alive(), "call_method hung after server disconnect") + self.assertIsInstance(outcome.get("exc"), ConnectionError) + + +if __name__ == "__main__": + unittest.main()