From 41307f90a6d1ebe097f3aef5b5b5e802dd78f543 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Mon, 2 Dec 2024 17:06:44 -0500 Subject: [PATCH] Fix Ice/timeout by making sure controller is connected (#3215) --- cpp/test/Ice/timeout/AllTests.cpp | 5 +- csharp/test/Ice/timeout/AllTests.cs | 10 ++-- .../main/java/test/Ice/timeout/AllTests.java | 40 +++++++++------ matlab/test/Ice/timeout/AllTests.m | 2 + php/test/Ice/timeout/Client.php | 2 + python/test/Ice/timeout/AllTests.py | 50 +++---------------- ruby/test/Ice/timeout/AllTests.rb | 1 + swift/test/Ice/timeout/AllTests.swift | 11 ++-- 8 files changed, 53 insertions(+), 68 deletions(-) diff --git a/cpp/test/Ice/timeout/AllTests.cpp b/cpp/test/Ice/timeout/AllTests.cpp index 0cee2a76460..f7740d93686 100644 --- a/cpp/test/Ice/timeout/AllTests.cpp +++ b/cpp/test/Ice/timeout/AllTests.cpp @@ -21,7 +21,7 @@ namespace // Establish connection with the given proxy (which might have a timeout // set and might sporadically fail on connection establishment if it's // too slow). The loop ensures that the connection is established by retrying - // in case we can a ConnectTimeoutException + // in case we get a ConnectTimeoutException // int nRetry = 10; while (--nRetry > 0) @@ -260,6 +260,9 @@ allTests(Test::TestHelper* helper) { ControllerPrx controller(helper->communicator(), "controller:" + helper->getTestEndpoint(1)); + // Make sure the controller is connected before we proceed. + connect(controller); + try { allTestsWithController(helper, controller); diff --git a/csharp/test/Ice/timeout/AllTests.cs b/csharp/test/Ice/timeout/AllTests.cs index 0c3f3c1db9f..afd0ae1cd97 100644 --- a/csharp/test/Ice/timeout/AllTests.cs +++ b/csharp/test/Ice/timeout/AllTests.cs @@ -26,10 +26,12 @@ private static Ice.Connection connect(ObjectPrx prx) public static async Task allTests(global::Test.TestHelper helper) { - Test.ControllerPrx controller = - Test.ControllerPrxHelper.checkedCast( - helper.communicator().stringToProxy("controller:" + helper.getTestEndpoint(1))); - test(controller != null); + Test.ControllerPrx controller = Test.ControllerPrxHelper.createProxy( + helper.communicator(), + "controller:" + helper.getTestEndpoint(1)); + + // Make sure the controller is connected before we proceed. + _ = connect(controller); try { diff --git a/java/test/src/main/java/test/Ice/timeout/AllTests.java b/java/test/src/main/java/test/Ice/timeout/AllTests.java index f6ce563f674..d399d3af7d2 100644 --- a/java/test/src/main/java/test/Ice/timeout/AllTests.java +++ b/java/test/src/main/java/test/Ice/timeout/AllTests.java @@ -20,12 +20,26 @@ private static void test(boolean b) { } } + private static com.zeroc.Ice.Connection connect(com.zeroc.Ice.ObjectPrx prx) { + int nRetry = 10; + while (--nRetry > 0) { + try { + prx.ice_getConnection(); + break; + } catch (com.zeroc.Ice.ConnectTimeoutException ex) { + // Can sporadically occur with slow machines + } + } + return prx.ice_getConnection(); // Establish connection + } + public static void allTests(test.TestHelper helper) { - ControllerPrx controller = - ControllerPrx.checkedCast( - helper.communicator() - .stringToProxy("controller:" + helper.getTestEndpoint(1))); - test(controller != null); + var controller = + ControllerPrx.createProxy( + helper.communicator(), "controller:" + helper.getTestEndpoint(1)); + + // Make sure the controller is connected before we proceed. + connect(controller); try { allTestsWithController(helper, controller); @@ -139,21 +153,17 @@ public static void allTestsWithController(test.TestHelper helper, ControllerPrx out.print("testing close timeout... "); out.flush(); { - // // This test wants to call some local methods while our connection is in the `Closing` - // state, - // before it eventually transitions to the `Closed` state due to hitting the close - // timeout. + // state, before it eventually transitions to the `Closed` state due to hitting the + // close timeout. // // However, in Java `close` blocks until the connection is closed. So, in order to - // access the - // `Closing` state, we initiate the close in a separate thread, wait 50ms to let the - // thread - // start the closure process, and hope that we're in the `Closing` state by then. - // + // access the `Closing` state, we initiate the close in a separate thread, wait 50ms to + // let the thread start the closure process, and hope that we're in the `Closing` state + // by then. // Get the connection, and put the OA in the `Hold` state. - var connection = timeout.ice_getConnection(); + var connection = connect(timeout); controller.holdAdapter(-1); // Initiate the connection closure. diff --git a/matlab/test/Ice/timeout/AllTests.m b/matlab/test/Ice/timeout/AllTests.m index 8155149e3ad..daaf2c4bfb6 100644 --- a/matlab/test/Ice/timeout/AllTests.m +++ b/matlab/test/Ice/timeout/AllTests.m @@ -25,6 +25,8 @@ function allTests(helper) import Test.*; controller = ControllerPrx(helper.communicator(), ['controller:', helper.getTestEndpoint(1)]); + AllTests.connect(controller); + try AllTests.allTestsWithController(helper, controller); catch ex diff --git a/php/test/Ice/timeout/Client.php b/php/test/Ice/timeout/Client.php index 0bd0c7d178b..b2ce73228db 100644 --- a/php/test/Ice/timeout/Client.php +++ b/php/test/Ice/timeout/Client.php @@ -40,6 +40,8 @@ function allTests($helper) $communicator, sprintf("controller:%s", $helper->getTestEndpoint(1))); + connect($controller); + try { allTestsWithController($helper, $controller); diff --git a/python/test/Ice/timeout/AllTests.py b/python/test/Ice/timeout/AllTests.py index 845aa08cae9..8d45cb4f402 100644 --- a/python/test/Ice/timeout/AllTests.py +++ b/python/test/Ice/timeout/AllTests.py @@ -5,47 +5,12 @@ import Ice import Test import sys -import threading def test(b): if not b: raise RuntimeError("test assertion failed") - -class CallbackBase: - def __init__(self): - self._called = False - self._cond = threading.Condition() - - def called(self): - with self._cond: - self._called = True - self._cond.notify() - - def check(self): - with self._cond: - while not self._called: - self._cond.wait() - self._called = False - return True - - -class Callback(CallbackBase): - def response(self): - self.called() - - def exception(self, ex): - test(False) - - def responseEx(self): - test(False) - - def exceptionEx(self, ex): - test(isinstance(ex, Ice.TimeoutException)) - self.called() - - def connect(prx): # Establish connection with the given proxy (which might have a timeout # set and might sporadically fail on connection establishment if it's @@ -64,7 +29,7 @@ def connect(prx): def allTests(helper, communicator): controller = Test.ControllerPrx(communicator, f"controller:{helper.getTestEndpoint(num=1)}") - test(controller is not None) + connect(controller) try: allTestsWithController(helper, communicator, controller) @@ -76,23 +41,20 @@ def allTests(helper, communicator): def allTestsWithController(helper, communicator, controller): - obj = Ice.ObjectPrx(communicator, f"timeout:{helper.getTestEndpoint()}") - - timeout = Test.TimeoutPrx.checkedCast(obj) - test(timeout is not None) + timeout = Test.TimeoutPrx(communicator, f"timeout:{helper.getTestEndpoint()}") sys.stdout.write("testing invocation timeout... ") sys.stdout.flush() - connection = obj.ice_getConnection() - to = Test.TimeoutPrx.uncheckedCast(obj.ice_invocationTimeout(100)) + connection = timeout.ice_getConnection() + to = timeout.ice_invocationTimeout(100) test(connection == to.ice_getConnection()) try: to.sleep(1000) test(False) except Ice.InvocationTimeoutException: pass - obj.ice_ping() - to = Test.TimeoutPrx.uncheckedCast(obj.ice_invocationTimeout(1000)) + timeout.ice_ping() + to = timeout.ice_invocationTimeout(1000) test(connection == to.ice_getConnection()) try: to.sleep(100) diff --git a/ruby/test/Ice/timeout/AllTests.rb b/ruby/test/Ice/timeout/AllTests.rb index dbde4e5c282..36ccc1625a1 100644 --- a/ruby/test/Ice/timeout/AllTests.rb +++ b/ruby/test/Ice/timeout/AllTests.rb @@ -18,6 +18,7 @@ def connect(prx) def allTests(helper, communicator) controller = Test::ControllerPrx.new(communicator, "controller:#{helper.getTestEndpoint(num:1)}") + connect(controller) begin allTestsWithController(helper, communicator, controller) rescue diff --git a/swift/test/Ice/timeout/AllTests.swift b/swift/test/Ice/timeout/AllTests.swift index 3eb453a63c3..e461005b04d 100644 --- a/swift/test/Ice/timeout/AllTests.swift +++ b/swift/test/Ice/timeout/AllTests.swift @@ -17,10 +17,13 @@ func connect(_ prx: Ice.ObjectPrx) async throws -> Ice.Connection { } public func allTests(helper: TestHelper) async throws { - let controller = try await checkedCast( - prx: helper.communicator().stringToProxy("controller:\(helper.getTestEndpoint(num: 1))")!, - type: ControllerPrx.self - )! + let controller = try makeProxy( + communicator: helper.communicator(), + proxyString: "controller:\(helper.getTestEndpoint(num: 1))", + type: ControllerPrx.self) + + try await connect(controller) + do { try await allTestsWithController(helper: helper, controller: controller) } catch {