From 66d97844b7b19f8643b23292a1577a0b4b9dc010 Mon Sep 17 00:00:00 2001 From: Benoit Foucher Date: Thu, 11 Jul 2019 13:10:39 +0200 Subject: Fixed another Java IceSSL failure on RHEL8, fixes #431 --- java/test/src/main/java/test/IceSSL/configuration/AllTests.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'java') diff --git a/java/test/src/main/java/test/IceSSL/configuration/AllTests.java b/java/test/src/main/java/test/IceSSL/configuration/AllTests.java index 1f978a03e7d..bfa01f4e882 100644 --- a/java/test/src/main/java/test/IceSSL/configuration/AllTests.java +++ b/java/test/src/main/java/test/IceSSL/configuration/AllTests.java @@ -1385,6 +1385,10 @@ public class AllTests { server.ice_ping(); } + catch(com.zeroc.Ice.SecurityException ex) + { + // Expected on systems that disable DSA (EL8) + } catch(com.zeroc.Ice.LocalException ex) { ex.printStackTrace(); @@ -1511,7 +1515,7 @@ public class AllTests } catch(com.zeroc.Ice.ConnectionLostException ex) { - // Expected on systems that disable DSA + // Expected on systems that disable DSA (EL8) } catch(com.zeroc.Ice.LocalException ex) { -- cgit v1.2.3 From 4192f6be40be8c17edda3bcb38f6774775e89ec9 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 11 Jul 2019 16:36:16 +0200 Subject: Update gradle wrapper used with Android test controller --- java/test/android/controller/build.gradle | 2 +- .../controller/gradle/wrapper/gradle-wrapper.jar | Bin 56177 -> 55190 bytes .../gradle/wrapper/gradle-wrapper.properties | 2 +- java/test/android/controller/gradlew | 2 +- java/test/android/controller/gradlew.bat | 2 +- java/test/android/controller/settings.gradle | 0 6 files changed, 4 insertions(+), 4 deletions(-) create mode 100644 java/test/android/controller/settings.gradle (limited to 'java') diff --git a/java/test/android/controller/build.gradle b/java/test/android/controller/build.gradle index 2e5f8ca9e75..e8f9040f821 100644 --- a/java/test/android/controller/build.gradle +++ b/java/test/android/controller/build.gradle @@ -11,7 +11,7 @@ buildscript { } } dependencies { - classpath 'com.android.tools.build:gradle:3.2.1' + classpath 'com.android.tools.build:gradle:3.4.2' classpath 'gradle.plugin.com.zeroc.gradle.ice-builder:slice:1.4.7' } } diff --git a/java/test/android/controller/gradle/wrapper/gradle-wrapper.jar b/java/test/android/controller/gradle/wrapper/gradle-wrapper.jar index 29953ea141f..87b738cbd05 100644 Binary files a/java/test/android/controller/gradle/wrapper/gradle-wrapper.jar and b/java/test/android/controller/gradle/wrapper/gradle-wrapper.jar differ diff --git a/java/test/android/controller/gradle/wrapper/gradle-wrapper.properties b/java/test/android/controller/gradle/wrapper/gradle-wrapper.properties index e0b3fb8d70b..558870dad58 100644 --- a/java/test/android/controller/gradle/wrapper/gradle-wrapper.properties +++ b/java/test/android/controller/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-5.1.1-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/java/test/android/controller/gradlew b/java/test/android/controller/gradlew index cccdd3d517f..af6708ff229 100755 --- a/java/test/android/controller/gradlew +++ b/java/test/android/controller/gradlew @@ -28,7 +28,7 @@ APP_NAME="Gradle" APP_BASE_NAME=`basename "$0"` # Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. -DEFAULT_JVM_OPTS="" +DEFAULT_JVM_OPTS='"-Xmx64m"' # Use the maximum available, or set MAX_FD != -1 to use that value. MAX_FD="maximum" diff --git a/java/test/android/controller/gradlew.bat b/java/test/android/controller/gradlew.bat index f9553162f12..6d57edc706c 100644 --- a/java/test/android/controller/gradlew.bat +++ b/java/test/android/controller/gradlew.bat @@ -14,7 +14,7 @@ set APP_BASE_NAME=%~n0 set APP_HOME=%DIRNAME% @rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. -set DEFAULT_JVM_OPTS= +set DEFAULT_JVM_OPTS="-Xmx64m" @rem Find java.exe if defined JAVA_HOME goto findJavaFromJavaHome diff --git a/java/test/android/controller/settings.gradle b/java/test/android/controller/settings.gradle new file mode 100644 index 00000000000..e69de29bb2d -- cgit v1.2.3 From bcd2c2d1b01e721be924b96247be86384ddc9738 Mon Sep 17 00:00:00 2001 From: Benoit Foucher Date: Thu, 11 Jul 2019 17:42:59 +0200 Subject: Fixed dispatcher interceptor bug #435 --- cpp/src/Ice/DispatchInterceptor.cpp | 13 +++++ cpp/src/Ice/Incoming.cpp | 2 + cpp/test/Ice/interceptor/AMDInterceptorI.cpp | 38 ++++++++++++- cpp/test/Ice/interceptor/Client.cpp | 55 ++++++++++++++++++ cpp/test/Ice/interceptor/InterceptorI.cpp | 39 ++++++++++++- csharp/src/Ice/DispatchInterceptor.cs | 18 +++++- csharp/test/Ice/interceptor/Client.cs | 56 ++++++++++++++++++ csharp/test/Ice/interceptor/InterceptorI.cs | 35 ++++++++++++ .../Ice/src/main/java/Ice/DispatchInterceptor.java | 13 +++++ .../java/test/Ice/interceptor/AMDInterceptorI.java | 37 ++++++++++++ .../src/main/java/test/Ice/interceptor/Client.java | 66 ++++++++++++++++++++++ .../java/test/Ice/interceptor/InterceptorI.java | 37 ++++++++++++ .../java/com/zeroc/Ice/DispatchInterceptor.java | 18 +++++- .../src/main/java/test/Ice/interceptor/Client.java | 66 ++++++++++++++++++++++ .../java/test/Ice/interceptor/InterceptorI.java | 36 ++++++++++++ 15 files changed, 525 insertions(+), 4 deletions(-) (limited to 'java') diff --git a/cpp/src/Ice/DispatchInterceptor.cpp b/cpp/src/Ice/DispatchInterceptor.cpp index 10457e990e9..ae12e77f900 100644 --- a/cpp/src/Ice/DispatchInterceptor.cpp +++ b/cpp/src/Ice/DispatchInterceptor.cpp @@ -21,4 +21,17 @@ Ice::DispatchInterceptor::_iceDispatch(IceInternal::Incoming& in, const Current& { return false; } + catch(const std::exception&) + { + // + // If the input parameters weren't read, make sure we skip them here. It's needed to read the + // encoding version used by the client to eventually marshal the user exception. It's also needed + // if we are dispatch a batch oneway request to read the next batch request. + // + if(in.getCurrent().encoding.major == 0 && in.getCurrent().encoding.minor == 0) + { + in.skipReadParams(); + } + throw; + } } diff --git a/cpp/src/Ice/Incoming.cpp b/cpp/src/Ice/Incoming.cpp index 1fce4e5f107..b7cc91488d2 100644 --- a/cpp/src/Ice/Incoming.cpp +++ b/cpp/src/Ice/Incoming.cpp @@ -59,6 +59,8 @@ IceInternal::IncomingBase::IncomingBase(Instance* instance, ResponseHandler* res _current.con = connection; #endif _current.requestId = requestId; + _current.encoding.major = 0; + _current.encoding.minor = 0; } IceInternal::IncomingBase::IncomingBase(IncomingBase& other) : diff --git a/cpp/test/Ice/interceptor/AMDInterceptorI.cpp b/cpp/test/Ice/interceptor/AMDInterceptorI.cpp index 33cfb5e0b38..e2e2d95b001 100644 --- a/cpp/test/Ice/interceptor/AMDInterceptorI.cpp +++ b/cpp/test/Ice/interceptor/AMDInterceptorI.cpp @@ -4,7 +4,7 @@ #include #include -#include +#include #include using namespace std; @@ -56,6 +56,24 @@ AMDInterceptorI::dispatch(Ice::Request& request) #endif Ice::Current& current = const_cast(request.getCurrent()); + + Ice::Context::const_iterator p = current.ctx.find("raiseBeforeDispatch"); + if(p != current.ctx.end()) + { + if(p->second == "user") + { + throw Test::InvalidInputException(); + } + else if(p->second == "notExist") + { + throw Ice::ObjectNotExistException(__FILE__, __LINE__); + } + else if(p->second == "system") + { + throw MySystemException(__FILE__, __LINE__); + } + } + _lastOperation = current.operation; if(_lastOperation == "amdAddWithRetry") @@ -105,6 +123,24 @@ AMDInterceptorI::dispatch(Ice::Request& request) #else _lastStatus = _servant->ice_dispatch(request, _defaultCb); #endif + + p = current.ctx.find("raiseAfterDispatch"); + if(p != current.ctx.end()) + { + if(p->second == "user") + { + throw Test::InvalidInputException(); + } + else if(p->second == "notExist") + { + throw Ice::ObjectNotExistException(__FILE__, __LINE__); + } + else if(p->second == "system") + { + throw MySystemException(__FILE__, __LINE__); + } + } + return _lastStatus; } diff --git a/cpp/test/Ice/interceptor/Client.cpp b/cpp/test/Ice/interceptor/Client.cpp index fd7c8314d26..dc93891e819 100644 --- a/cpp/test/Ice/interceptor/Client.cpp +++ b/cpp/test/Ice/interceptor/Client.cpp @@ -35,6 +35,7 @@ private: void runTest(const Test::MyObjectPrxPtr&, const InterceptorIPtr&); void runAmdTest(const Test::MyObjectPrxPtr&, const AMDInterceptorIPtr&); + void testInterceptorExceptions(const Test::MyObjectPrx&); }; void @@ -169,6 +170,10 @@ Client::runTest(const Test::MyObjectPrxPtr& prx, const InterceptorIPtr& intercep test(interceptor->getLastOperation() == "amdAdd"); test(!interceptor->getLastStatus()); cout << "ok" << endl; + + cout << "testing exceptions raised by the interceptor... " << flush; + testInterceptorExceptions(prx); + cout << "ok" << endl; } void @@ -234,6 +239,56 @@ Client::runAmdTest(const Test::MyObjectPrxPtr& prx, const AMDInterceptorIPtr& in test(!interceptor->getLastStatus()); test(dynamic_cast(interceptor->getException()) != 0); cout << "ok" << endl; + + cout << "testing exceptions raised by the interceptor... " << flush; + testInterceptorExceptions(prx); + cout << "ok" << endl; +} + +void +Client::testInterceptorExceptions(const Test::MyObjectPrx& prx) +{ + vector > exceptions; + exceptions.push_back(make_pair("raiseBeforeDispatch", "user")); + exceptions.push_back(make_pair("raiseBeforeDispatch", "notExist")); + exceptions.push_back(make_pair("raiseBeforeDispatch", "system")); + exceptions.push_back(make_pair("raiseAfterDispatch", "user")); + exceptions.push_back(make_pair("raiseAfterDispatch", "notExist")); + exceptions.push_back(make_pair("raiseAfterDispatch", "system")); + for(vector >::const_iterator p = exceptions.begin(); p != exceptions.end(); ++p) + { + Ice::Context ctx; + ctx[p->first] = p->second; + try + { + prx->ice_ping(ctx); + test(false); + } + catch(const Ice::UnknownUserException&) + { + test(p->second == "user"); + } + catch(const Ice::ObjectNotExistException&) + { + test(p->second == "notExist"); + } + catch(const Ice::UnknownException&) + { + test(p->second == "system"); // non-collocated + } + catch(const MySystemException&) + { + test(p->second == "system"); // collocated + } + { + Ice::ObjectPrx batch = prx->ice_batchOneway(); + batch->ice_ping(ctx); + batch->ice_ping(); + batch->ice_flushBatchRequests(); + } + } + // Force the last batch request to be dispatched by the server thread using invocation timeouts + prx->ice_invocationTimeout(10000)->ice_ping(); } DEFINE_TEST(Client) diff --git a/cpp/test/Ice/interceptor/InterceptorI.cpp b/cpp/test/Ice/interceptor/InterceptorI.cpp index 04b60f7a8f6..171aaf0c25a 100644 --- a/cpp/test/Ice/interceptor/InterceptorI.cpp +++ b/cpp/test/Ice/interceptor/InterceptorI.cpp @@ -3,7 +3,7 @@ // #include -#include +#include #include using namespace std; @@ -18,6 +18,24 @@ bool InterceptorI::dispatch(Ice::Request& request) { Ice::Current& current = const_cast(request.getCurrent()); + + Ice::Context::const_iterator p = current.ctx.find("raiseBeforeDispatch"); + if(p != current.ctx.end()) + { + if(p->second == "user") + { + throw Test::InvalidInputException(); + } + else if(p->second == "notExist") + { + throw Ice::ObjectNotExistException(__FILE__, __LINE__); + } + else if(p->second == "system") + { + throw MySystemException(__FILE__, __LINE__); + } + } + _lastOperation = current.operation; if(_lastOperation == "addWithRetry") @@ -39,7 +57,26 @@ InterceptorI::dispatch(Ice::Request& request) current.ctx["retry"] = "no"; } + _lastStatus = _servant->ice_dispatch(request); + + p = current.ctx.find("raiseAfterDispatch"); + if(p != current.ctx.end()) + { + if(p->second == "user") + { + throw Test::InvalidInputException(); + } + else if(p->second == "notExist") + { + throw Ice::ObjectNotExistException(__FILE__, __LINE__); + } + else if(p->second == "system") + { + throw MySystemException(__FILE__, __LINE__); + } + } + return _lastStatus; } diff --git a/csharp/src/Ice/DispatchInterceptor.cs b/csharp/src/Ice/DispatchInterceptor.cs index 665b504ef3e..58a63672bd3 100644 --- a/csharp/src/Ice/DispatchInterceptor.cs +++ b/csharp/src/Ice/DispatchInterceptor.cs @@ -26,7 +26,23 @@ namespace Ice public override System.Threading.Tasks.Task iceDispatch(IceInternal.Incoming inc, Current current) { - return dispatch(inc); + try + { + return dispatch(inc); + } + catch(Exception) + { + // + // If the input parameters weren't read, make sure we skip them here. It's needed to read the + // encoding version used by the client to eventually marshal the user exception. It's also needed + // if we are dispatch a batch oneway request to read the next batch request. + // + if(current.encoding == null || (current.encoding.major == 0 && current.encoding.minor == 0)) + { + inc.skipReadParams(); + } + throw; + } } } } diff --git a/csharp/test/Ice/interceptor/Client.cs b/csharp/test/Ice/interceptor/Client.cs index a459ee8fb65..6c8b59cfd52 100644 --- a/csharp/test/Ice/interceptor/Client.cs +++ b/csharp/test/Ice/interceptor/Client.cs @@ -3,6 +3,7 @@ // using System; +using System.Collections.Generic; using Test; namespace Ice @@ -89,6 +90,11 @@ namespace Ice test(interceptor.getLastOperation().Equals("badSystemAdd")); test(!interceptor.getLastStatus()); output.WriteLine("ok"); + + output.Write("testing exceptions raised by the interceptor... "); + output.Flush(); + testInterceptorExceptions(prx); + output.WriteLine("ok"); } private void runAmdTest(Test.MyObjectPrx prx, InterceptorI interceptor) @@ -163,6 +169,11 @@ namespace Ice test(interceptor.getLastOperation().Equals("amdBadSystemAdd")); test(interceptor.getLastStatus()); output.WriteLine("ok"); + + output.Write("testing exceptions raised by the interceptor... "); + output.Flush(); + testInterceptorExceptions(prx); + output.WriteLine("ok"); } public override void run(string[] args) @@ -206,6 +217,51 @@ namespace Ice { return TestDriver.runTest(args); } + + private void testInterceptorExceptions(Test.MyObjectPrx prx) + { + var exceptions = new List>(); + exceptions.Add(new Tuple("raiseBeforeDispatch", "user")); + exceptions.Add(new Tuple("raiseBeforeDispatch", "notExist")); + exceptions.Add(new Tuple("raiseBeforeDispatch", "system")); + exceptions.Add(new Tuple("raiseAfterDispatch", "user")); + exceptions.Add(new Tuple("raiseAfterDispatch", "notExist")); + exceptions.Add(new Tuple("raiseAfterDispatch", "system")); + foreach(var e in exceptions) + { + var ctx = new Dictionary(); + ctx.Add(e.Item1, e.Item2); + try + { + prx.ice_ping(ctx); + test(false); + } + catch(Ice.UnknownUserException) + { + test(e.Item2.Equals("user")); + } + catch(Ice.ObjectNotExistException) + { + test(e.Item2.Equals("notExist")); + } + catch(Ice.UnknownException) + { + test(e.Item2.Equals("system")); // non-collocated + } + catch(MySystemException) + { + test(e.Item2.Equals("system")); // collocated + } + { + Ice.ObjectPrx batch = prx.ice_batchOneway(); + batch.ice_ping(ctx); + batch.ice_ping(); + batch.ice_flushBatchRequests(); + } + } + // Force the last batch request to be dispatched by the server thread using invocation timeouts + prx.ice_invocationTimeout(10000).ice_ping(); + } } } } diff --git a/csharp/test/Ice/interceptor/InterceptorI.cs b/csharp/test/Ice/interceptor/InterceptorI.cs index 19d33bbbb58..522d5059cb5 100644 --- a/csharp/test/Ice/interceptor/InterceptorI.cs +++ b/csharp/test/Ice/interceptor/InterceptorI.cs @@ -29,6 +29,24 @@ namespace Ice dispatch(Ice.Request request) { Ice.Current current = request.getCurrent(); + + string context; + if(current.ctx.TryGetValue("raiseBeforeDispatch", out context)) + { + if(context.Equals("user")) + { + throw new Test.InvalidInputException(); + } + else if(context.Equals("notExist")) + { + throw new Ice.ObjectNotExistException(); + } + else if(context.Equals("system")) + { + throw new MySystemException(); + } + } + lastOperation_ = current.operation; if(lastOperation_.Equals("addWithRetry") || lastOperation_.Equals("amdAddWithRetry")) @@ -60,6 +78,23 @@ namespace Ice var task = servant_.ice_dispatch(request); lastStatus_ = task != null; + + if(current.ctx.TryGetValue("raiseAfterDispatch", out context)) + { + if(context.Equals("user")) + { + throw new Test.InvalidInputException(); + } + else if(context.Equals("notExist")) + { + throw new Ice.ObjectNotExistException(); + } + else if(context.Equals("system")) + { + throw new MySystemException(); + } + } + return task; } diff --git a/java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java b/java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java index 8735ff7eea6..3935de43b3e 100644 --- a/java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java +++ b/java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java @@ -43,5 +43,18 @@ public abstract class DispatchInterceptor extends ObjectImpl { return false; } + catch(java.lang.Throwable ex) + { + // + // If the input parameters weren't read, make sure we skip them here. It's needed to read the + // encoding version used by the client to eventually marshal the user exception. It's also needed + // if we are dispatch a batch oneway request to read the next batch request. + // + if(current.encoding == null || (current.encoding.major == 0 && current.encoding.minor == 0)) + { + in.skipReadParams(); + } + throw ex; + } } } diff --git a/java-compat/test/src/main/java/test/Ice/interceptor/AMDInterceptorI.java b/java-compat/test/src/main/java/test/Ice/interceptor/AMDInterceptorI.java index ffade5fbac4..a6a855b0595 100644 --- a/java-compat/test/src/main/java/test/Ice/interceptor/AMDInterceptorI.java +++ b/java-compat/test/src/main/java/test/Ice/interceptor/AMDInterceptorI.java @@ -5,6 +5,7 @@ package test.Ice.interceptor; import test.Ice.interceptor.Test.RetryException; +import test.Ice.interceptor.Test.InvalidInputException; // // A dispatch interceptor with special handling for AMD requests @@ -23,6 +24,24 @@ class AMDInterceptorI extends InterceptorI implements Ice.DispatchInterceptorAsy throws Ice.UserException { Ice.Current current = request.getCurrent(); + + String context = current.ctx.get("raiseBeforeDispatch"); + if(context != null) + { + if(context.equals("user")) + { + throw new InvalidInputException(); + } + else if(context.equals("notExist")) + { + throw new Ice.ObjectNotExistException(); + } + else if(context.equals("system")) + { + throw new MySystemException(); + } + } + _lastOperation = current.operation; if(_lastOperation.equals("amdAddWithRetry")) @@ -53,6 +72,24 @@ class AMDInterceptorI extends InterceptorI implements Ice.DispatchInterceptorAsy } _lastStatus = _servant.ice_dispatch(request, this); + + context = current.ctx.get("raiseAfterDispatch"); + if(context != null) + { + if(context.equals("user")) + { + throw new InvalidInputException(); + } + else if(context.equals("notExist")) + { + throw new Ice.ObjectNotExistException(); + } + else if(context.equals("system")) + { + throw new MySystemException(); + } + } + return _lastStatus; } diff --git a/java-compat/test/src/main/java/test/Ice/interceptor/Client.java b/java-compat/test/src/main/java/test/Ice/interceptor/Client.java index d029404145e..a4e5c436947 100644 --- a/java-compat/test/src/main/java/test/Ice/interceptor/Client.java +++ b/java-compat/test/src/main/java/test/Ice/interceptor/Client.java @@ -96,6 +96,11 @@ public class Client extends test.TestHelper test(interceptor.getLastOperation().equals("amdAdd")); test(!interceptor.getLastStatus()); out.println("ok"); + + out.print("testing exceptions raised by the interceptor... "); + out.flush(); + testInterceptorExceptions(prx); + out.println("ok"); } private void @@ -169,6 +174,11 @@ public class Client extends test.TestHelper test(!interceptor.getLastStatus()); test(interceptor.getException() instanceof MySystemException); out.println("ok"); + + out.print("testing exceptions raised by the interceptor... "); + out.flush(); + testInterceptorExceptions(prx); + out.println("ok"); } public void run(String[] args) @@ -210,4 +220,60 @@ public class Client extends test.TestHelper runAmdTest(prxForAMD, amdInterceptor, out); } } + + private class ExceptionPoint + { + public ExceptionPoint(String point, String exception) + { + this.point = point; + this.exception = exception; + } + public String point; + public String exception; + }; + + private void testInterceptorExceptions(MyObjectPrx prx) + { + java.util.List exceptions = new java.util.ArrayList(); + exceptions.add(new ExceptionPoint("raiseBeforeDispatch", "user")); + exceptions.add(new ExceptionPoint("raiseBeforeDispatch", "notExist")); + exceptions.add(new ExceptionPoint("raiseBeforeDispatch", "system")); + exceptions.add(new ExceptionPoint("raiseAfterDispatch", "user")); + exceptions.add(new ExceptionPoint("raiseAfterDispatch", "notExist")); + exceptions.add(new ExceptionPoint("raiseAfterDispatch", "system")); + for(ExceptionPoint e : exceptions) + { + java.util.Map ctx = new java.util.HashMap(); + ctx.put(e.point, e.exception); + try + { + prx.ice_ping(ctx); + test(false); + } + catch(Ice.UnknownUserException ex) + { + test(e.exception.equals("user")); + } + catch(Ice.ObjectNotExistException ex) + { + test(e.exception.equals("notExist")); + } + catch(Ice.UnknownException ex) + { + test(e.exception.equals("system")); // non-collocated + } + catch(MySystemException ex) + { + test(e.exception.equals("system")); // collocated + } + { + Ice.ObjectPrx batch = prx.ice_batchOneway(); + batch.ice_ping(ctx); + batch.ice_ping(); + batch.ice_flushBatchRequests(); + } + } + // Force the last batch request to be dispatched by the server thread using invocation timeouts + prx.ice_invocationTimeout(10000).ice_ping(); + } } diff --git a/java-compat/test/src/main/java/test/Ice/interceptor/InterceptorI.java b/java-compat/test/src/main/java/test/Ice/interceptor/InterceptorI.java index 547234e705b..0c067d65d66 100644 --- a/java-compat/test/src/main/java/test/Ice/interceptor/InterceptorI.java +++ b/java-compat/test/src/main/java/test/Ice/interceptor/InterceptorI.java @@ -5,6 +5,7 @@ package test.Ice.interceptor; import test.Ice.interceptor.Test.RetryException; +import test.Ice.interceptor.Test.InvalidInputException; class InterceptorI extends Ice.DispatchInterceptor { @@ -28,6 +29,24 @@ class InterceptorI extends Ice.DispatchInterceptor throws Ice.UserException { Ice.Current current = request.getCurrent(); + + String context = current.ctx.get("raiseBeforeDispatch"); + if(context != null) + { + if(context.equals("user")) + { + throw new InvalidInputException(); + } + else if(context.equals("notExist")) + { + throw new Ice.ObjectNotExistException(); + } + else if(context.equals("system")) + { + throw new MySystemException(); + } + } + _lastOperation = current.operation; if(_lastOperation.equals("addWithRetry")) @@ -51,6 +70,24 @@ class InterceptorI extends Ice.DispatchInterceptor } _lastStatus = _servant.ice_dispatch(request); + + context = current.ctx.get("raiseAfterDispatch"); + if(context != null) + { + if(context.equals("user")) + { + throw new InvalidInputException(); + } + else if(context.equals("notExist")) + { + throw new Ice.ObjectNotExistException(); + } + else if(context.equals("system")) + { + throw new MySystemException(); + } + } + return _lastStatus; } diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java b/java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java index d825707630e..b5f23bb9c27 100644 --- a/java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java +++ b/java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java @@ -36,6 +36,22 @@ public abstract class DispatchInterceptor implements com.zeroc.Ice.Object public CompletionStage _iceDispatch(com.zeroc.IceInternal.Incoming in, Current current) throws UserException { - return dispatch(in); + try + { + return dispatch(in); + } + catch(java.lang.Throwable ex) + { + // + // If the input parameters weren't read, make sure we skip them here. It's needed to read the + // encoding version used by the client to eventually marshal the user exception. It's also needed + // if we are dispatch a batch oneway request to read the next batch request. + // + if(current.encoding == null || (current.encoding.major == 0 && current.encoding.minor == 0)) + { + in.skipReadParams(); + } + throw ex; + } } } diff --git a/java/test/src/main/java/test/Ice/interceptor/Client.java b/java/test/src/main/java/test/Ice/interceptor/Client.java index 9bd89a14661..b45f9eec748 100644 --- a/java/test/src/main/java/test/Ice/interceptor/Client.java +++ b/java/test/src/main/java/test/Ice/interceptor/Client.java @@ -89,6 +89,11 @@ public class Client extends test.TestHelper test(interceptor.getLastOperation().equals("badSystemAdd")); test(!interceptor.getLastStatus()); out.println("ok"); + + out.print("testing exceptions raised by the interceptor... "); + out.flush(); + testInterceptorExceptions(prx); + out.println("ok"); } private void runAmdTest(MyObjectPrx prx, InterceptorI interceptor, PrintWriter out) @@ -159,6 +164,11 @@ public class Client extends test.TestHelper test(interceptor.getLastOperation().equals("amdBadSystemAdd")); test(interceptor.getLastStatus()); out.println("ok"); + + out.print("testing exceptions raised by the interceptor... "); + out.flush(); + testInterceptorExceptions(prx); + out.println("ok"); } public void run(String[] args) @@ -197,4 +207,60 @@ public class Client extends test.TestHelper runAmdTest(prx, interceptor, out); } } + + private class ExceptionPoint + { + public ExceptionPoint(String point, String exception) + { + this.point = point; + this.exception = exception; + } + public String point; + public String exception; + }; + + private void testInterceptorExceptions(MyObjectPrx prx) + { + java.util.List exceptions = new java.util.ArrayList<>(); + exceptions.add(new ExceptionPoint("raiseBeforeDispatch", "user")); + exceptions.add(new ExceptionPoint("raiseBeforeDispatch", "notExist")); + exceptions.add(new ExceptionPoint("raiseBeforeDispatch", "system")); + exceptions.add(new ExceptionPoint("raiseAfterDispatch", "user")); + exceptions.add(new ExceptionPoint("raiseAfterDispatch", "notExist")); + exceptions.add(new ExceptionPoint("raiseAfterDispatch", "system")); + for(ExceptionPoint e : exceptions) + { + java.util.Map ctx = new java.util.HashMap<>(); + ctx.put(e.point, e.exception); + try + { + prx.ice_ping(ctx); + test(false); + } + catch(com.zeroc.Ice.UnknownUserException ex) + { + test(e.exception.equals("user")); + } + catch(com.zeroc.Ice.ObjectNotExistException ex) + { + test(e.exception.equals("notExist")); + } + catch(com.zeroc.Ice.UnknownException ex) + { + test(e.exception.equals("system")); // non-collocated + } + catch(MySystemException ex) + { + test(e.exception.equals("system")); // collocated + } + { + com.zeroc.Ice.ObjectPrx batch = prx.ice_batchOneway(); + batch.ice_ping(ctx); + batch.ice_ping(); + batch.ice_flushBatchRequests(); + } + } + // Force the last batch request to be dispatched by the server thread using invocation timeouts + prx.ice_invocationTimeout(10000).ice_ping(); + } } diff --git a/java/test/src/main/java/test/Ice/interceptor/InterceptorI.java b/java/test/src/main/java/test/Ice/interceptor/InterceptorI.java index 2437717ecb1..5ea248e1c43 100644 --- a/java/test/src/main/java/test/Ice/interceptor/InterceptorI.java +++ b/java/test/src/main/java/test/Ice/interceptor/InterceptorI.java @@ -9,6 +9,7 @@ import java.util.concurrent.CompletionStage; import com.zeroc.Ice.OutputStream; import test.Ice.interceptor.Test.RetryException; +import test.Ice.interceptor.Test.InvalidInputException; class InterceptorI extends com.zeroc.Ice.DispatchInterceptor { @@ -30,6 +31,24 @@ class InterceptorI extends com.zeroc.Ice.DispatchInterceptor throws com.zeroc.Ice.UserException { com.zeroc.Ice.Current current = request.getCurrent(); + + String context = current.ctx.get("raiseBeforeDispatch"); + if(context != null) + { + if(context.equals("user")) + { + throw new InvalidInputException(); + } + else if(context.equals("notExist")) + { + throw new com.zeroc.Ice.ObjectNotExistException(); + } + else if(context.equals("system")) + { + throw new MySystemException(); + } + } + _lastOperation = current.operation; if(_lastOperation.equals("addWithRetry") || _lastOperation.equals("amdAddWithRetry")) @@ -54,6 +73,23 @@ class InterceptorI extends com.zeroc.Ice.DispatchInterceptor CompletionStage f = _servant.ice_dispatch(request); _lastStatus = f != null; + + context = current.ctx.get("raiseAfterDispatch"); + if(context != null) + { + if(context.equals("user")) + { + throw new InvalidInputException(); + } + else if(context.equals("notExist")) + { + throw new com.zeroc.Ice.ObjectNotExistException(); + } + else if(context.equals("system")) + { + throw new MySystemException(); + } + } return f; } -- cgit v1.2.3 From 3184f261238e75470e18fd9441524eddcb4847df Mon Sep 17 00:00:00 2001 From: Benoit Foucher Date: Thu, 11 Jul 2019 18:19:52 +0200 Subject: Fixed C++11 issue with previous Ice/interceptor test fix --- cpp/src/Ice/DispatchInterceptor.cpp | 2 +- cpp/test/Ice/interceptor/Client.cpp | 6 +++--- csharp/src/Ice/DispatchInterceptor.cs | 2 +- java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java | 2 +- java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) (limited to 'java') diff --git a/cpp/src/Ice/DispatchInterceptor.cpp b/cpp/src/Ice/DispatchInterceptor.cpp index ae12e77f900..78e79311631 100644 --- a/cpp/src/Ice/DispatchInterceptor.cpp +++ b/cpp/src/Ice/DispatchInterceptor.cpp @@ -26,7 +26,7 @@ Ice::DispatchInterceptor::_iceDispatch(IceInternal::Incoming& in, const Current& // // If the input parameters weren't read, make sure we skip them here. It's needed to read the // encoding version used by the client to eventually marshal the user exception. It's also needed - // if we are dispatch a batch oneway request to read the next batch request. + // if we dispatch a batch oneway request to read the next batch request. // if(in.getCurrent().encoding.major == 0 && in.getCurrent().encoding.minor == 0) { diff --git a/cpp/test/Ice/interceptor/Client.cpp b/cpp/test/Ice/interceptor/Client.cpp index dc93891e819..d47dc7abc22 100644 --- a/cpp/test/Ice/interceptor/Client.cpp +++ b/cpp/test/Ice/interceptor/Client.cpp @@ -35,7 +35,7 @@ private: void runTest(const Test::MyObjectPrxPtr&, const InterceptorIPtr&); void runAmdTest(const Test::MyObjectPrxPtr&, const AMDInterceptorIPtr&); - void testInterceptorExceptions(const Test::MyObjectPrx&); + void testInterceptorExceptions(const Test::MyObjectPrxPtr&); }; void @@ -246,7 +246,7 @@ Client::runAmdTest(const Test::MyObjectPrxPtr& prx, const AMDInterceptorIPtr& in } void -Client::testInterceptorExceptions(const Test::MyObjectPrx& prx) +Client::testInterceptorExceptions(const Test::MyObjectPrxPtr& prx) { vector > exceptions; exceptions.push_back(make_pair("raiseBeforeDispatch", "user")); @@ -281,7 +281,7 @@ Client::testInterceptorExceptions(const Test::MyObjectPrx& prx) test(p->second == "system"); // collocated } { - Ice::ObjectPrx batch = prx->ice_batchOneway(); + Ice::ObjectPrxPtr batch = prx->ice_batchOneway(); batch->ice_ping(ctx); batch->ice_ping(); batch->ice_flushBatchRequests(); diff --git a/csharp/src/Ice/DispatchInterceptor.cs b/csharp/src/Ice/DispatchInterceptor.cs index 58a63672bd3..54e39251ac6 100644 --- a/csharp/src/Ice/DispatchInterceptor.cs +++ b/csharp/src/Ice/DispatchInterceptor.cs @@ -35,7 +35,7 @@ namespace Ice // // If the input parameters weren't read, make sure we skip them here. It's needed to read the // encoding version used by the client to eventually marshal the user exception. It's also needed - // if we are dispatch a batch oneway request to read the next batch request. + // if we dispatch a batch oneway request to read the next batch request. // if(current.encoding == null || (current.encoding.major == 0 && current.encoding.minor == 0)) { diff --git a/java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java b/java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java index 3935de43b3e..355e81dd354 100644 --- a/java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java +++ b/java-compat/src/Ice/src/main/java/Ice/DispatchInterceptor.java @@ -48,7 +48,7 @@ public abstract class DispatchInterceptor extends ObjectImpl // // If the input parameters weren't read, make sure we skip them here. It's needed to read the // encoding version used by the client to eventually marshal the user exception. It's also needed - // if we are dispatch a batch oneway request to read the next batch request. + // if we dispatch a batch oneway request to read the next batch request. // if(current.encoding == null || (current.encoding.major == 0 && current.encoding.minor == 0)) { diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java b/java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java index b5f23bb9c27..8523bdc929d 100644 --- a/java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java +++ b/java/src/Ice/src/main/java/com/zeroc/Ice/DispatchInterceptor.java @@ -45,7 +45,7 @@ public abstract class DispatchInterceptor implements com.zeroc.Ice.Object // // If the input parameters weren't read, make sure we skip them here. It's needed to read the // encoding version used by the client to eventually marshal the user exception. It's also needed - // if we are dispatch a batch oneway request to read the next batch request. + // if we dispatch a batch oneway request to read the next batch request. // if(current.encoding == null || (current.encoding.major == 0 && current.encoding.minor == 0)) { -- cgit v1.2.3 From 6dff3d8c9376596d9a1e8064dbf226386156c6d3 Mon Sep 17 00:00:00 2001 From: Jose Date: Mon, 15 Jul 2019 13:31:51 +0200 Subject: Update gradle wrapper to latest 4.10 patch release --- java-compat/gradle/wrapper/gradle-wrapper.jar | Bin 54712 -> 56177 bytes .../gradle/wrapper/gradle-wrapper.properties | 2 +- java/gradle/wrapper/gradle-wrapper.jar | Bin 56177 -> 56177 bytes java/gradle/wrapper/gradle-wrapper.properties | 2 +- 4 files changed, 2 insertions(+), 2 deletions(-) (limited to 'java') diff --git a/java-compat/gradle/wrapper/gradle-wrapper.jar b/java-compat/gradle/wrapper/gradle-wrapper.jar index ed88a042a28..94336fcae91 100644 Binary files a/java-compat/gradle/wrapper/gradle-wrapper.jar and b/java-compat/gradle/wrapper/gradle-wrapper.jar differ diff --git a/java-compat/gradle/wrapper/gradle-wrapper.properties b/java-compat/gradle/wrapper/gradle-wrapper.properties index fb7ef980f26..290541c7386 100644 --- a/java-compat/gradle/wrapper/gradle-wrapper.properties +++ b/java-compat/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists +distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.3-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-bin.zip diff --git a/java/gradle/wrapper/gradle-wrapper.jar b/java/gradle/wrapper/gradle-wrapper.jar index 29953ea141f..94336fcae91 100644 Binary files a/java/gradle/wrapper/gradle-wrapper.jar and b/java/gradle/wrapper/gradle-wrapper.jar differ diff --git a/java/gradle/wrapper/gradle-wrapper.properties b/java/gradle/wrapper/gradle-wrapper.properties index e0b3fb8d70b..290541c7386 100644 --- a/java/gradle/wrapper/gradle-wrapper.properties +++ b/java/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.3-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -- cgit v1.2.3 From 1b20aaa7889f7cc376fba7c5da8da8b7dce57f58 Mon Sep 17 00:00:00 2001 From: Benoit Foucher Date: Tue, 16 Jul 2019 17:32:44 +0200 Subject: Fixed potential metrics test failure, fixes #217 --- cpp/test/Ice/metrics/AllTests.cpp | 12 +++------ csharp/test/Ice/metrics/AllTests.cs | 18 +++---------- .../src/main/java/test/Ice/metrics/AllTests.java | 12 +++------ .../src/main/java/test/Ice/metrics/AllTests.java | 12 +++------ objective-c/test/Ice/metrics/AllTests.m | 30 +++------------------- 5 files changed, 20 insertions(+), 64 deletions(-) (limited to 'java') diff --git a/cpp/test/Ice/metrics/AllTests.cpp b/cpp/test/Ice/metrics/AllTests.cpp index 3e3e40cf19e..1beef435bb5 100644 --- a/cpp/test/Ice/metrics/AllTests.cpp +++ b/cpp/test/Ice/metrics/AllTests.cpp @@ -965,7 +965,7 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv) cout << "testing invocation metrics... " << flush; props["IceMX.Metrics.View.Map.Invocation.GroupBy"] = "operation"; - props["IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy"] = "localPort"; + props["IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy"] = "id"; props["IceMX.Metrics.View.Map.Invocation.Map.Collocated.GroupBy"] = "parent"; updateProps(clientProps, serverProps, update.get(), props, "Invocation"); test(serverMetrics->getMetricsView("View", timestamp)["Invocation"].empty()); @@ -1323,13 +1323,9 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv) if(!collocated) { im1 = ICE_DYNAMIC_CAST(IceMX::InvocationMetrics, map["fail"]); - test(im1->current <= 1 && im1->total == 3 && im1->failures == 3 && im1->retry == 3 && im1->remotes.size() == 6); - test(im1->remotes[0]->current == 0 && im1->remotes[0]->total == 1 && im1->remotes[0]->failures == 1); - test(im1->remotes[1]->current == 0 && im1->remotes[1]->total == 1 && im1->remotes[1]->failures == 1); - test(im1->remotes[2]->current == 0 && im1->remotes[2]->total == 1 && im1->remotes[2]->failures == 1); - test(im1->remotes[3]->current == 0 && im1->remotes[3]->total == 1 && im1->remotes[3]->failures == 1); - test(im1->remotes[4]->current == 0 && im1->remotes[4]->total == 1 && im1->remotes[4]->failures == 1); - test(im1->remotes[5]->current == 0 && im1->remotes[5]->total == 1 && im1->remotes[5]->failures == 1); + test(im1->current <= 1 && im1->total == 3 && im1->failures == 3 && im1->retry == 3 && im1->remotes.size() == 1); + rim1 = ICE_DYNAMIC_CAST(IceMX::ChildInvocationMetrics, im1->remotes[0]); + test(rim1->current == 0 && rim1->total == 6 && rim1->failures == 6); checkFailure(clientMetrics, "Invocation", im1->id, "::Ice::ConnectionLostException", 3); } diff --git a/csharp/test/Ice/metrics/AllTests.cs b/csharp/test/Ice/metrics/AllTests.cs index 387a276b248..31326ba9c53 100644 --- a/csharp/test/Ice/metrics/AllTests.cs +++ b/csharp/test/Ice/metrics/AllTests.cs @@ -914,7 +914,7 @@ public class AllTests : Test.AllTests // Tests for twoway // props["IceMX.Metrics.View.Map.Invocation.GroupBy"] = "operation"; - props["IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy"] = "localPort"; + props["IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy"] = "id"; props["IceMX.Metrics.View.Map.Invocation.Map.Collocated.GroupBy"] = "id"; updateProps(clientProps, serverProps, update, props, "Invocation"); test(serverMetrics.getMetricsView("View", out timestamp)["Invocation"].Length == 0); @@ -1071,19 +1071,9 @@ public class AllTests : Test.AllTests if(!collocated) { im1 = (IceMX.InvocationMetrics)map["fail"]; - if(!(im1.current <= 1 && im1.total == 3 && im1.failures == 3 && im1.retry == 3 && im1.remotes.Length == 6)) - { - System.Console.Error.WriteLine("current: " + im1.current + " total: " + im1.total + - " failures: " + im1.failures + " retry: " + im1.retry + - " remotes: " + im1.remotes.Length); - test(false); - } - test(im1.remotes[0].current == 0 && im1.remotes[0].total == 1 && im1.remotes[0].failures == 1); - test(im1.remotes[1].current == 0 && im1.remotes[1].total == 1 && im1.remotes[1].failures == 1); - test(im1.remotes[2].current == 0 && im1.remotes[2].total == 1 && im1.remotes[2].failures == 1); - test(im1.remotes[3].current == 0 && im1.remotes[3].total == 1 && im1.remotes[3].failures == 1); - test(im1.remotes[4].current == 0 && im1.remotes[4].total == 1 && im1.remotes[4].failures == 1); - test(im1.remotes[5].current == 0 && im1.remotes[5].total == 1 && im1.remotes[5].failures == 1); + test(im1.current <= 1 && im1.total == 3 && im1.failures == 3 && im1.retry == 3 && im1.remotes.Length == 1); + rim1 = (IceMX.ChildInvocationMetrics)(collocated ? im1.collocated[0] : im1.remotes[0]); + test(rim1.current == 0 && rim1.total == 6 && rim1.failures == 6); checkFailure(clientMetrics, "Invocation", im1.id, "::Ice::ConnectionLostException", 3, output); } diff --git a/java-compat/test/src/main/java/test/Ice/metrics/AllTests.java b/java-compat/test/src/main/java/test/Ice/metrics/AllTests.java index c4843c606b4..376c72a76af 100644 --- a/java-compat/test/src/main/java/test/Ice/metrics/AllTests.java +++ b/java-compat/test/src/main/java/test/Ice/metrics/AllTests.java @@ -977,7 +977,7 @@ public class AllTests out.flush(); props.put("IceMX.Metrics.View.Map.Invocation.GroupBy", "operation"); - props.put("IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy", "localPort"); + props.put("IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy", "id"); props.put("IceMX.Metrics.View.Map.Invocation.Map.Collocated.GroupBy", "id"); updateProps(clientProps, serverProps, update, props, "Invocation"); test(serverMetrics.getMetricsView("View", timestamp).get("Invocation").length == 0); @@ -1142,13 +1142,9 @@ public class AllTests if(!collocated) { im1 = (IceMX.InvocationMetrics)map.get("fail"); - test(im1.current <= 1 && im1.total == 3 && im1.failures == 3 && im1.retry == 3 && im1.remotes.length == 6); - test(im1.remotes[0].current == 0 && im1.remotes[0].total == 1 && im1.remotes[0].failures == 1); - test(im1.remotes[1].current == 0 && im1.remotes[1].total == 1 && im1.remotes[1].failures == 1); - test(im1.remotes[2].current == 0 && im1.remotes[2].total == 1 && im1.remotes[2].failures == 1); - test(im1.remotes[3].current == 0 && im1.remotes[3].total == 1 && im1.remotes[3].failures == 1); - test(im1.remotes[4].current == 0 && im1.remotes[4].total == 1 && im1.remotes[4].failures == 1); - test(im1.remotes[5].current == 0 && im1.remotes[5].total == 1 && im1.remotes[5].failures == 1); + test(im1.current <= 1 && im1.total == 3 && im1.failures == 3 && im1.retry == 3 && im1.remotes.length == 1); + rim1 = (IceMX.ChildInvocationMetrics)im1.remotes[0]; + test(rim1.current == 0 && rim1.total == 6 && rim1.failures == 6); checkFailure(clientMetrics, "Invocation", im1.id, "::Ice::ConnectionLostException", 3, out); } diff --git a/java/test/src/main/java/test/Ice/metrics/AllTests.java b/java/test/src/main/java/test/Ice/metrics/AllTests.java index e72cb7586a9..c6035f6548e 100644 --- a/java/test/src/main/java/test/Ice/metrics/AllTests.java +++ b/java/test/src/main/java/test/Ice/metrics/AllTests.java @@ -911,7 +911,7 @@ public class AllTests out.flush(); props.put("IceMX.Metrics.View.Map.Invocation.GroupBy", "operation"); - props.put("IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy", "localPort"); + props.put("IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy", "id"); props.put("IceMX.Metrics.View.Map.Invocation.Map.Collocated.GroupBy", "id"); updateProps(clientProps, serverProps, props, "Invocation"); test(serverMetrics.getMetricsView("View").returnValue.get("Invocation").length == 0); @@ -1094,13 +1094,9 @@ public class AllTests if(!collocated) { im1 = (InvocationMetrics)map.get("fail"); - test(im1.current <= 1 && im1.total == 3 && im1.failures == 3 && im1.retry == 3 && im1.remotes.length == 6); - test(im1.remotes[0].current == 0 && im1.remotes[0].total == 1 && im1.remotes[0].failures == 1); - test(im1.remotes[1].current == 0 && im1.remotes[1].total == 1 && im1.remotes[1].failures == 1); - test(im1.remotes[2].current == 0 && im1.remotes[2].total == 1 && im1.remotes[2].failures == 1); - test(im1.remotes[3].current == 0 && im1.remotes[3].total == 1 && im1.remotes[3].failures == 1); - test(im1.remotes[4].current == 0 && im1.remotes[4].total == 1 && im1.remotes[4].failures == 1); - test(im1.remotes[5].current == 0 && im1.remotes[5].total == 1 && im1.remotes[5].failures == 1); + test(im1.current <= 1 && im1.total == 3 && im1.failures == 3 && im1.retry == 3 && im1.remotes.length == 1); + rim1 = (ChildInvocationMetrics)im1.remotes[0]; + test(rim1.current == 0 && rim1.total == 6 && rim1.failures == 6); checkFailure(clientMetrics, "Invocation", im1.id, "::Ice::ConnectionLostException", 3, out); } diff --git a/objective-c/test/Ice/metrics/AllTests.m b/objective-c/test/Ice/metrics/AllTests.m index 6e4049afb62..986d86cd3c0 100644 --- a/objective-c/test/Ice/metrics/AllTests.m +++ b/objective-c/test/Ice/metrics/AllTests.m @@ -917,7 +917,7 @@ metricsAllTests(id communicator) tprintf("testing invocation metrics... "); [props setObject:@"operation" forKey:@"IceMX.Metrics.View.Map.Invocation.GroupBy"]; - [props setObject:@"localPort" forKey:@"IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy"]; + [props setObject:@"id" forKey:@"IceMX.Metrics.View.Map.Invocation.Map.Remote.GroupBy"]; updateProps(clientProps, serverProps, update, props, @"Invocation"); test([[[serverMetrics getMetricsView:@"View" timestamp:×tamp] objectForKey:@"Invocation"] count] == 0); @@ -1077,31 +1077,9 @@ metricsAllTests(id communicator) checkFailure(clientMetrics, @"Invocation", im1.id_, @"::Ice::UnknownException", 3); im1 = (ICEMXInvocationMetrics*)[map objectForKey:@"fail"]; - test(im1.current <= 1 && im1.total == 3 && im1.failures == 3 && im1.retry == 3 && [im1.remotes count] == 6); - - test(((ICEMXMetrics*)[im1.remotes objectAtIndex:0]).current == 0 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:0]).total == 1 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:0]).failures == 1); - - test(((ICEMXMetrics*)[im1.remotes objectAtIndex:1]).current == 0 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:1]).total == 1 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:1]).failures == 1); - - test(((ICEMXMetrics*)[im1.remotes objectAtIndex:2]).current == 0 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:2]).total == 1 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:2]).failures == 1); - - test(((ICEMXMetrics*)[im1.remotes objectAtIndex:3]).current == 0 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:3]).total == 1 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:3]).failures == 1); - - test(((ICEMXMetrics*)[im1.remotes objectAtIndex:4]).current == 0 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:4]).total == 1 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:4]).failures == 1); - - test(((ICEMXMetrics*)[im1.remotes objectAtIndex:5]).current == 0 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:5]).total == 1 && - ((ICEMXMetrics*)[im1.remotes objectAtIndex:5]).failures == 1); + test(im1.current <= 1 && im1.total == 3 && im1.failures == 3 && im1.retry == 3 && [im1.remotes count] == 1); + rim1 = (ICEMXRemoteMetrics*)[im1.remotes objectAtIndex:0]; + test(rim1.current == 0 && rim1.total == 6 && rim1.failures == 6); checkFailure(clientMetrics, @"Invocation", im1.id_, @"::Ice::ConnectionLostException", 3); -- cgit v1.2.3 From 26725c376f9e674a22dc3ec2a18ea1e2a461a790 Mon Sep 17 00:00:00 2001 From: Benoit Foucher Date: Tue, 16 Jul 2019 18:52:11 +0200 Subject: Fixed dispatch interceptor test threading issue, fixes #446 --- cpp/test/Ice/interceptor/Client.cpp | 7 +++++-- csharp/test/Ice/interceptor/Client.cs | 7 +++++-- java-compat/test/src/main/java/test/Ice/interceptor/Client.java | 7 +++++-- java/test/src/main/java/test/Ice/interceptor/Client.java | 7 +++++-- 4 files changed, 20 insertions(+), 8 deletions(-) (limited to 'java') diff --git a/cpp/test/Ice/interceptor/Client.cpp b/cpp/test/Ice/interceptor/Client.cpp index d47dc7abc22..191215fd59d 100644 --- a/cpp/test/Ice/interceptor/Client.cpp +++ b/cpp/test/Ice/interceptor/Client.cpp @@ -285,10 +285,13 @@ Client::testInterceptorExceptions(const Test::MyObjectPrxPtr& prx) batch->ice_ping(ctx); batch->ice_ping(); batch->ice_flushBatchRequests(); + + // Force the last batch request to be dispatched by the server thread using invocation timeouts + // This is required to preven threading issue with the test interceptor implementation which + // isn't thread safe + prx->ice_invocationTimeout(10000)->ice_ping(); } } - // Force the last batch request to be dispatched by the server thread using invocation timeouts - prx->ice_invocationTimeout(10000)->ice_ping(); } DEFINE_TEST(Client) diff --git a/csharp/test/Ice/interceptor/Client.cs b/csharp/test/Ice/interceptor/Client.cs index 6c8b59cfd52..2345891a324 100644 --- a/csharp/test/Ice/interceptor/Client.cs +++ b/csharp/test/Ice/interceptor/Client.cs @@ -257,10 +257,13 @@ namespace Ice batch.ice_ping(ctx); batch.ice_ping(); batch.ice_flushBatchRequests(); + + // Force the last batch request to be dispatched by the server thread using invocation timeouts + // This is required to preven threading issue with the test interceptor implementation which + // isn't thread safe + prx.ice_invocationTimeout(10000).ice_ping(); } } - // Force the last batch request to be dispatched by the server thread using invocation timeouts - prx.ice_invocationTimeout(10000).ice_ping(); } } } diff --git a/java-compat/test/src/main/java/test/Ice/interceptor/Client.java b/java-compat/test/src/main/java/test/Ice/interceptor/Client.java index a4e5c436947..0661f9e2117 100644 --- a/java-compat/test/src/main/java/test/Ice/interceptor/Client.java +++ b/java-compat/test/src/main/java/test/Ice/interceptor/Client.java @@ -271,9 +271,12 @@ public class Client extends test.TestHelper batch.ice_ping(ctx); batch.ice_ping(); batch.ice_flushBatchRequests(); + + // Force the last batch request to be dispatched by the server thread using invocation timeouts + // This is required to preven threading issue with the test interceptor implementation which + // isn't thread safe + prx.ice_invocationTimeout(10000).ice_ping(); } } - // Force the last batch request to be dispatched by the server thread using invocation timeouts - prx.ice_invocationTimeout(10000).ice_ping(); } } diff --git a/java/test/src/main/java/test/Ice/interceptor/Client.java b/java/test/src/main/java/test/Ice/interceptor/Client.java index b45f9eec748..dafae39f792 100644 --- a/java/test/src/main/java/test/Ice/interceptor/Client.java +++ b/java/test/src/main/java/test/Ice/interceptor/Client.java @@ -258,9 +258,12 @@ public class Client extends test.TestHelper batch.ice_ping(ctx); batch.ice_ping(); batch.ice_flushBatchRequests(); + + // Force the last batch request to be dispatched by the server thread using invocation timeouts + // This is required to preven threading issue with the test interceptor implementation which + // isn't thread safe + prx.ice_invocationTimeout(10000).ice_ping(); } } - // Force the last batch request to be dispatched by the server thread using invocation timeouts - prx.ice_invocationTimeout(10000).ice_ping(); } } -- cgit v1.2.3 From 2c036ca6ca7f39b6987135839a1a899cd080877d Mon Sep 17 00:00:00 2001 From: Benoit Foucher Date: Thu, 18 Jul 2019 18:09:15 +0200 Subject: Fixed non-thread safe AMD dispatch, fixes #448 (#449) The caching of the output stream which was added back to solve #414 made the dispatch of AMD requests non thread-safe if a dispatch interceptor was installed and if the interceptor retried the dispatch. Multiple continuation could run concurrently and eventually re-use the cached output stream to marshal multiple responses. --- cpp/test/Ice/interceptor/AMDInterceptorI.cpp | 9 +++ cpp/test/Ice/interceptor/Client.cpp | 10 +++ cpp/test/Ice/interceptor/MyObjectI.cpp | 36 +++++++-- csharp/Makefile | 8 +- csharp/src/Ice/Incoming.cs | 94 +++++++++++++++++----- csharp/src/Ice/Object.cs | 8 +- csharp/test/Ice/interceptor/Client.cs | 12 +++ csharp/test/Ice/interceptor/InterceptorI.cs | 9 +++ .../java/test/Ice/interceptor/AMDInterceptorI.java | 9 +++ .../src/main/java/test/Ice/interceptor/Client.java | 10 +++ .../main/java/test/Ice/interceptor/MyObjectI.java | 13 ++- .../Ice/src/main/java/com/zeroc/Ice/Blobject.java | 2 +- .../src/main/java/com/zeroc/Ice/BlobjectAsync.java | 6 +- .../main/java/com/zeroc/IceInternal/Incoming.java | 69 ++++++++++++---- .../src/main/java/test/Ice/interceptor/Client.java | 10 +++ .../java/test/Ice/interceptor/InterceptorI.java | 9 +++ 16 files changed, 260 insertions(+), 54 deletions(-) (limited to 'java') diff --git a/cpp/test/Ice/interceptor/AMDInterceptorI.cpp b/cpp/test/Ice/interceptor/AMDInterceptorI.cpp index e2e2d95b001..158cf2736fa 100644 --- a/cpp/test/Ice/interceptor/AMDInterceptorI.cpp +++ b/cpp/test/Ice/interceptor/AMDInterceptorI.cpp @@ -103,6 +103,15 @@ AMDInterceptorI::dispatch(Ice::Request& request) current.ctx["retry"] = "no"; } + else if(current.ctx.find("retry") != current.ctx.end() && current.ctx["retry"] == "yes") + { + // + // Retry the dispatch to ensure that abandoning the result of the dispatch + // works fine and is thread-safe + // + _servant->ice_dispatch(request); + _servant->ice_dispatch(request); + } #ifdef ICE_CPP11_MAPPING _lastStatus = _servant->ice_dispatch(request, []() { return true; }, [this](exception_ptr ex) { diff --git a/cpp/test/Ice/interceptor/Client.cpp b/cpp/test/Ice/interceptor/Client.cpp index 191215fd59d..02317c19d2c 100644 --- a/cpp/test/Ice/interceptor/Client.cpp +++ b/cpp/test/Ice/interceptor/Client.cpp @@ -189,6 +189,16 @@ Client::runAmdTest(const Test::MyObjectPrxPtr& prx, const AMDInterceptorIPtr& in test(prx->amdAddWithRetry(33, 12) == 45); test(interceptor->getLastOperation() == "amdAddWithRetry"); test(!interceptor->getLastStatus()); + { + Ice::Context ctx; + ctx["retry"] = "yes"; + for(int i = 0; i < 10; ++i) + { + test(prx->amdAdd(33, 12, ctx) == 45); + test(interceptor->getLastOperation() == "amdAdd"); + test(!interceptor->getLastStatus()); + } + } cout << "ok" << endl; cout << "testing user exception... " << flush; try diff --git a/cpp/test/Ice/interceptor/MyObjectI.cpp b/cpp/test/Ice/interceptor/MyObjectI.cpp index 8604cd7007f..f3164ac4426 100644 --- a/cpp/test/Ice/interceptor/MyObjectI.cpp +++ b/cpp/test/Ice/interceptor/MyObjectI.cpp @@ -83,13 +83,22 @@ MyObjectI::amdAddAsync(int x, int y, function response, function, - const Ice::Current&) + const Ice::Current& current) { + Ice::Context::const_iterator p = current.ctx.find("retry"); + bool retry = p != current.ctx.end(); std::thread t( - [x, y, response]() + [x, y, response, retry]() { this_thread::sleep_for(chrono::milliseconds(10)); - response(x + y); + try + { + response(x + y); + } + catch(const Ice::ResponseSentException&) + { + test(retry); + } }); t.detach(); } @@ -200,31 +209,42 @@ MyObjectI::amdBadSystemAddAsync(int, } #else void -MyObjectI::amdAdd_async(const Test::AMD_MyObject_amdAddPtr& cb, int x, int y, const Ice::Current&) +MyObjectI::amdAdd_async(const Test::AMD_MyObject_amdAddPtr& cb, int x, int y, const Ice::Current& current) { class ThreadI : public Thread { public: - ThreadI(const Test::AMD_MyObject_amdAddPtr& cb, int x, int y) : + ThreadI(const Test::AMD_MyObject_amdAddPtr& cb, int x, int y, bool retry) : _cb(cb), _x(x), - _y(y) + _y(y), + _retry(retry) { } void run() { ThreadControl::sleep(Time::milliSeconds(10)); - _cb->ice_response(_x + _y); + try + { + _cb->ice_response(_x + _y); + } + catch(const Ice::ResponseSentException&) + { + test(_retry); + } } private: Test::AMD_MyObject_amdAddPtr _cb; int _x; int _y; + bool _retry; }; - ThreadPtr thread = new ThreadI(cb, x, y); + Ice::Context::const_iterator p = current.ctx.find("retry"); + bool retry = p != current.ctx.end(); + ThreadPtr thread = new ThreadI(cb, x, y, retry); thread->start().detach(); } diff --git a/csharp/Makefile b/csharp/Makefile index f0dc5088e0c..33c59414242 100644 --- a/csharp/Makefile +++ b/csharp/Makefile @@ -2,14 +2,16 @@ # Copyright (c) ZeroC, Inc. All rights reserved. # +DOTNETARGS = $(if $(filter $(OPTIMIZE),no),/p:Configuration=Debug) + all: - dotnet msbuild msbuild/ice.proj /m + dotnet msbuild msbuild/ice.proj $(DOTNETARGS) /m tests: - dotnet msbuild msbuild/ice.proj /m + dotnet msbuild msbuild/ice.proj $(DOTNETARGS) /m srcs: - dotnet msbuild msbuild/ice.proj /t:BuildDist /m + dotnet msbuild msbuild/ice.proj $(DOTNETARGS) /t:BuildDist /m distclean clean: dotnet msbuild msbuild/ice.proj /t:Clean /m diff --git a/csharp/src/Ice/Incoming.cs b/csharp/src/Ice/Incoming.cs index 621099d773b..c467f67c349 100644 --- a/csharp/src/Ice/Incoming.cs +++ b/csharp/src/Ice/Incoming.cs @@ -268,17 +268,27 @@ namespace IceInternal { if(task == null) { - _os = startWriteParams(); - write(_os, default(R)); - endWriteParams(_os); - return null; // Response is cached in the Incoming to not have to create unecessary Task + // + // Write default constructed response if no task is provided + // + var os = startWriteParams(); + write(os, default(R)); + endWriteParams(os); + return setResult(os); } else { + var cached = getAndClearCachedOutputStream(); // If an output stream is cached, re-use it + + // + // NOTE: it's important that the continuation doesn't mutate the Incoming state to + // guarantee thread-safety. Multiple continuations can execute concurrently if the + // user installed a dispatch interceptor and the dispatch is retried. + // return task.ContinueWith((Task t) => { var result = t.GetAwaiter().GetResult(); - var os = startWriteParams(); + var os = startWriteParams(cached); write(os, result); endWriteParams(os); return Task.FromResult(os); @@ -290,15 +300,24 @@ namespace IceInternal { if(task == null) { - _os = writeEmptyParams(); - return null; + // + // Write response if no task is provided + // + return setResult(writeEmptyParams()); } else { + var cached = getAndClearCachedOutputStream(); // If an output stream is cached, re-use it + + // + // NOTE: it's important that the continuation doesn't mutate the Incoming state to + // guarantee thread-safety. Multiple continuations can execute concurrently if the + // user installed a dispatch interceptor and the dispatch is retried. + // return task.ContinueWith((Task t) => { t.GetAwaiter().GetResult(); - return Task.FromResult(writeEmptyParams()); + return Task.FromResult(writeEmptyParams(cached)); }, TaskContinuationOptions.ExecuteSynchronously).Unwrap(); } } @@ -307,11 +326,15 @@ namespace IceInternal { if(task == null) { - _os = default(T).getOutputStream(_current); - return null; // Response is cached in the Incoming to not have to create unecessary Task + return setResult(default(T).getOutputStream(_current)); } else { + // + // NOTE: it's important that the continuation doesn't mutate the Incoming state to + // guarantee thread-safety. Multiple continuations can execute concurrently if the + // user installed a dispatch interceptor and the dispatch is retried. + // return task.ContinueWith((Task t) => { return Task.FromResult(t.GetAwaiter().GetResult().getOutputStream(_current)); @@ -432,6 +455,20 @@ namespace IceInternal _format = format; } + public Ice.OutputStream getAndClearCachedOutputStream() + { + if(_response) + { + var cached = _os; + _os = null; + return cached; + } + else + { + return null; // Don't consume unnecessarily the output stream if we are dispatching a oneway request + } + } + static public Ice.OutputStream createResponseOutputStream(Ice.Current current) { var os = new Ice.OutputStream(current.adapter.getCommunicator(), Ice.Util.currentProtocolEncoding); @@ -441,17 +478,18 @@ namespace IceInternal return os; } - public Ice.OutputStream startWriteParams() + private Ice.OutputStream startWriteParams(Ice.OutputStream os) { if(!_response) { throw new Ice.MarshalException("can't marshal out parameters for oneway dispatch"); } - // If there's an output stream set, re-use it for the response - var os = _os == null ? new Ice.OutputStream(_instance, Ice.Util.currentProtocolEncoding) : _os; + if(os == null) // Create the output stream if none is provided + { + os = new Ice.OutputStream(_instance, Ice.Util.currentProtocolEncoding); + } Debug.Assert(os.pos() == 0); - _os = null; os.writeBlob(Protocol.replyHdr); os.writeInt(_current.requestId); os.writeByte(ReplyStatus.replyOK); @@ -459,6 +497,11 @@ namespace IceInternal return os; } + public Ice.OutputStream startWriteParams() + { + return startWriteParams(getAndClearCachedOutputStream()); + } + public void endWriteParams(Ice.OutputStream os) { if(_response) @@ -467,14 +510,15 @@ namespace IceInternal } } - public Ice.OutputStream writeEmptyParams() + private Ice.OutputStream writeEmptyParams(Ice.OutputStream os) { if(_response) { - // If there's an output stream set, re-use it for the response - var os = _os == null ? new Ice.OutputStream(_instance, Ice.Util.currentProtocolEncoding) : _os; + if(os == null) // Create the output stream if none is provided + { + os = new Ice.OutputStream(_instance, Ice.Util.currentProtocolEncoding); + } Debug.Assert(os.pos() == 0); - _os = null; os.writeBlob(Protocol.replyHdr); os.writeInt(_current.requestId); os.writeByte(ReplyStatus.replyOK); @@ -487,7 +531,12 @@ namespace IceInternal } } - public Ice.OutputStream writeParamEncaps(byte[] v, bool ok) + public Ice.OutputStream writeEmptyParams() + { + return writeEmptyParams(getAndClearCachedOutputStream()); + } + + public Ice.OutputStream writeParamEncaps(Ice.OutputStream os, byte[] v, bool ok) { if(!ok && _observer != null) { @@ -496,10 +545,11 @@ namespace IceInternal if(_response) { - // If there's an output stream set, re-use it for the response - var os = _os == null ? new Ice.OutputStream(_instance, Ice.Util.currentProtocolEncoding) : _os; + if(os == null) // Create the output stream if none is provided + { + os = new Ice.OutputStream(_instance, Ice.Util.currentProtocolEncoding); + } Debug.Assert(os.pos() == 0); - _os = null; os.writeBlob(Protocol.replyHdr); os.writeInt(_current.requestId); os.writeByte(ok ? ReplyStatus.replyOK : ReplyStatus.replyUserException); diff --git a/csharp/src/Ice/Object.cs b/csharp/src/Ice/Object.cs index dd9913ba5f3..78f776e671b 100644 --- a/csharp/src/Ice/Object.cs +++ b/csharp/src/Ice/Object.cs @@ -310,7 +310,7 @@ namespace Ice byte[] inEncaps = inS.readParamEncaps(); byte[] outEncaps; bool ok = ice_invoke(inEncaps, out outEncaps, current); - inS.setResult(inS.writeParamEncaps(outEncaps, ok)); + inS.setResult(inS.writeParamEncaps(inS.getAndClearCachedOutputStream(), outEncaps, ok)); return null; } } @@ -323,10 +323,12 @@ namespace Ice public override Task iceDispatch(IceInternal.Incoming inS, Current current) { byte[] inEncaps = inS.readParamEncaps(); - return ice_invokeAsync(inEncaps, current).ContinueWith((Task t) => + var task = ice_invokeAsync(inEncaps, current); + var cached = inS.getAndClearCachedOutputStream(); + return task.ContinueWith((Task t) => { var ret = t.GetAwaiter().GetResult(); - return Task.FromResult(inS.writeParamEncaps(ret.outEncaps, ret.returnValue)); + return Task.FromResult(inS.writeParamEncaps(cached, ret.outEncaps, ret.returnValue)); }).Unwrap(); } } diff --git a/csharp/test/Ice/interceptor/Client.cs b/csharp/test/Ice/interceptor/Client.cs index 2345891a324..efdc4a6db83 100644 --- a/csharp/test/Ice/interceptor/Client.cs +++ b/csharp/test/Ice/interceptor/Client.cs @@ -114,6 +114,18 @@ namespace Ice test(prx.amdAddWithRetry(33, 12) == 45); test(interceptor.getLastOperation().Equals("amdAddWithRetry")); test(interceptor.getLastStatus()); + + { + var ctx = new Dictionary(); + ctx.Add("retry", "yes"); + for(int i = 0; i < 10; ++i) + { + test(prx.amdAdd(33, 12, ctx) == 45); + test(interceptor.getLastOperation().Equals("amdAdd")); + test(interceptor.getLastStatus()); + } + } + output.WriteLine("ok"); output.Write("testing user exception... "); diff --git a/csharp/test/Ice/interceptor/InterceptorI.cs b/csharp/test/Ice/interceptor/InterceptorI.cs index 522d5059cb5..c3f88efd13a 100644 --- a/csharp/test/Ice/interceptor/InterceptorI.cs +++ b/csharp/test/Ice/interceptor/InterceptorI.cs @@ -75,6 +75,15 @@ namespace Ice current.ctx["retry"] = "no"; } + else if(current.ctx.TryGetValue("retry", out context) && context.Equals("yes")) + { + // + // Retry the dispatch to ensure that abandoning the result of the dispatch + // works fine and is thread-safe + // + servant_.ice_dispatch(request); + servant_.ice_dispatch(request); + } var task = servant_.ice_dispatch(request); lastStatus_ = task != null; diff --git a/java-compat/test/src/main/java/test/Ice/interceptor/AMDInterceptorI.java b/java-compat/test/src/main/java/test/Ice/interceptor/AMDInterceptorI.java index a6a855b0595..db208419d44 100644 --- a/java-compat/test/src/main/java/test/Ice/interceptor/AMDInterceptorI.java +++ b/java-compat/test/src/main/java/test/Ice/interceptor/AMDInterceptorI.java @@ -70,6 +70,15 @@ class AMDInterceptorI extends InterceptorI implements Ice.DispatchInterceptorAsy request.getCurrent().ctx.put("retry", "no"); } + else if(current.ctx.get("retry") != null && current.ctx.get("retry").equals("yes")) + { + // + // Retry the dispatch to ensure that abandoning the result of the dispatch + // works fine and is thread-safe + // + _servant.ice_dispatch(request); + _servant.ice_dispatch(request); + } _lastStatus = _servant.ice_dispatch(request, this); diff --git a/java-compat/test/src/main/java/test/Ice/interceptor/Client.java b/java-compat/test/src/main/java/test/Ice/interceptor/Client.java index 0661f9e2117..2c95ccfdc21 100644 --- a/java-compat/test/src/main/java/test/Ice/interceptor/Client.java +++ b/java-compat/test/src/main/java/test/Ice/interceptor/Client.java @@ -119,6 +119,16 @@ public class Client extends test.TestHelper test(prx.amdAddWithRetry(33, 12) == 45); test(interceptor.getLastOperation().equals("amdAddWithRetry")); test(!interceptor.getLastStatus()); + { + java.util.Map ctx = new java.util.HashMap<>(); + ctx.put("retry", "yes"); + for(int i = 0; i < 10; ++i) + { + test(prx.amdAdd(33, 12, ctx) == 45); + test(interceptor.getLastOperation().equals("amdAdd")); + test(!interceptor.getLastStatus()); + } + } out.println("ok"); out.print("testing user exception... "); out.flush(); diff --git a/java-compat/test/src/main/java/test/Ice/interceptor/MyObjectI.java b/java-compat/test/src/main/java/test/Ice/interceptor/MyObjectI.java index 1f60cb0adb8..cebd1882ccd 100644 --- a/java-compat/test/src/main/java/test/Ice/interceptor/MyObjectI.java +++ b/java-compat/test/src/main/java/test/Ice/interceptor/MyObjectI.java @@ -65,6 +65,7 @@ class MyObjectI extends _MyObjectDisp public void amdAdd_async(final AMD_MyObject_amdAdd cb, final int x, final int y, Ice.Current current) { + final boolean retry = current.ctx.get("retry") != null && current.ctx.get("retry").equals("yes"); Thread thread = new Thread() { @Override @@ -78,7 +79,17 @@ class MyObjectI extends _MyObjectDisp catch(InterruptedException e) { } - cb.ice_response(x + y); + try + { + cb.ice_response(x + y); + } + catch(Ice.ResponseSentException ex) + { + if(!retry) + { + throw ex; + } + } } }; diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/Blobject.java b/java/src/Ice/src/main/java/com/zeroc/Ice/Blobject.java index e23f5e7e6a5..9d3637badaf 100644 --- a/java/src/Ice/src/main/java/com/zeroc/Ice/Blobject.java +++ b/java/src/Ice/src/main/java/com/zeroc/Ice/Blobject.java @@ -39,6 +39,6 @@ public interface Blobject extends com.zeroc.Ice.Object { byte[] inEncaps = in.readParamEncaps(); com.zeroc.Ice.Object.Ice_invokeResult r = ice_invoke(inEncaps, current); - return in.setResult(in.writeParamEncaps(r.outParams, r.returnValue)); + return in.setResult(in.writeParamEncaps(in.getAndClearCachedOutputStream(), r.outParams, r.returnValue)); } } diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/BlobjectAsync.java b/java/src/Ice/src/main/java/com/zeroc/Ice/BlobjectAsync.java index cb9ddadfd16..6a148071905 100644 --- a/java/src/Ice/src/main/java/com/zeroc/Ice/BlobjectAsync.java +++ b/java/src/Ice/src/main/java/com/zeroc/Ice/BlobjectAsync.java @@ -45,7 +45,9 @@ public interface BlobjectAsync extends com.zeroc.Ice.Object { byte[] inEncaps = in.readParamEncaps(); CompletableFuture f = new CompletableFuture<>(); - ice_invokeAsync(inEncaps, current).whenComplete((result, ex) -> + CompletionStage s = ice_invokeAsync(inEncaps, current); + final OutputStream cached = in.getAndClearCachedOutputStream(); // If an output stream is cached, re-use it + s.whenComplete((result, ex) -> { if(ex != null) { @@ -53,7 +55,7 @@ public interface BlobjectAsync extends com.zeroc.Ice.Object } else { - f.complete(in.writeParamEncaps(result.outParams, result.returnValue)); + f.complete(in.writeParamEncaps(cached, result.outParams, result.returnValue)); } }); return f; diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/Incoming.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/Incoming.java index dc5a60d390a..f8d899ae365 100644 --- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/Incoming.java +++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/Incoming.java @@ -271,6 +271,13 @@ final public class Incoming implements com.zeroc.Ice.Request public CompletionStage setResultFuture(CompletionStage f, Write write) { + final OutputStream cached = getAndClearCachedOutputStream(); // If an output stream is cached, re-use it + + // + // NOTE: it's important that the continuation doesn't mutate the Incoming state to + // guarantee thread-safety. Multiple continuations can execute concurrently if the + // user installed a dispatch interceptor and the dispatch is retried. + // final CompletableFuture r = new CompletableFuture(); f.whenComplete((result, ex) -> { @@ -280,7 +287,7 @@ final public class Incoming implements com.zeroc.Ice.Request } else { - OutputStream os = startWriteParams(); + OutputStream os = startWriteParams(cached); write.write(os, result); endWriteParams(os); r.complete(os); @@ -291,6 +298,13 @@ final public class Incoming implements com.zeroc.Ice.Request public CompletionStage setResultFuture(CompletionStage f) { + final OutputStream cached = getAndClearCachedOutputStream(); // If an output stream is cached, re-use it + + // + // NOTE: it's important that the continuation doesn't mutate the Incoming state to + // guarantee thread-safety. Multiple continuations can execute concurrently if the + // user installed a dispatch interceptor and the dispatch is retried. + // final CompletableFuture r = new CompletableFuture(); f.whenComplete((result, ex) -> { @@ -300,7 +314,7 @@ final public class Incoming implements com.zeroc.Ice.Request } else { - r.complete(writeEmptyParams()); + r.complete(writeEmptyParams(cached)); } }); return r; @@ -448,6 +462,20 @@ final public class Incoming implements com.zeroc.Ice.Request _format = format; } + public OutputStream getAndClearCachedOutputStream() + { + if(_response) + { + OutputStream cached = _os; + _os = null; + return cached; + } + else + { + return null; // Don't consume unnecessarily the output stream if we are dispatching a oneway request + } + } + static public OutputStream createResponseOutputStream(Current current) { OutputStream os = new OutputStream(current.adapter.getCommunicator(), Protocol.currentProtocolEncoding); @@ -457,17 +485,18 @@ final public class Incoming implements com.zeroc.Ice.Request return os; } - public OutputStream startWriteParams() + private OutputStream startWriteParams(OutputStream os) { if(!_response) { throw new com.zeroc.Ice.MarshalException("can't marshal out parameters for oneway dispatch"); } - // If there's an output stream set, re-use it for the response - OutputStream os = _os == null ? new OutputStream(_instance, Protocol.currentProtocolEncoding) : _os; + if(os == null) // Create the output stream if none is provided + { + os = new OutputStream(_instance, Protocol.currentProtocolEncoding); + } assert(os.pos() == 0); - _os = null; os.writeBlob(Protocol.replyHdr); os.writeInt(_current.requestId); os.writeByte(ReplyStatus.replyOK); @@ -475,6 +504,11 @@ final public class Incoming implements com.zeroc.Ice.Request return os; } + public OutputStream startWriteParams() + { + return startWriteParams(getAndClearCachedOutputStream()); + } + public void endWriteParams(OutputStream os) { if(_response) @@ -483,14 +517,15 @@ final public class Incoming implements com.zeroc.Ice.Request } } - public OutputStream writeEmptyParams() + private OutputStream writeEmptyParams(OutputStream os) { if(_response) { - // If there's an output stream set, re-use it for the response - OutputStream os = _os == null ? new OutputStream(_instance, Protocol.currentProtocolEncoding) : _os; + if(os == null) // Create the output stream if none is provided + { + os = new OutputStream(_instance, Protocol.currentProtocolEncoding); + } assert(os.pos() == 0); - _os = null; os.writeBlob(Protocol.replyHdr); os.writeInt(_current.requestId); os.writeByte(ReplyStatus.replyOK); @@ -503,7 +538,12 @@ final public class Incoming implements com.zeroc.Ice.Request } } - public OutputStream writeParamEncaps(byte[] v, boolean ok) + public OutputStream writeEmptyParams() + { + return writeEmptyParams(getAndClearCachedOutputStream()); + } + + public OutputStream writeParamEncaps(OutputStream os, byte[] v, boolean ok) { if(!ok && _observer != null) { @@ -512,10 +552,11 @@ final public class Incoming implements com.zeroc.Ice.Request if(_response) { - // If there's an output stream set, re-use it for the response - OutputStream os = _os == null ? new OutputStream(_instance, Protocol.currentProtocolEncoding) : _os; + if(os == null) // Create the output stream if none is provided + { + os = new OutputStream(_instance, Protocol.currentProtocolEncoding); + } assert(os.pos() == 0); - _os = null; os.writeBlob(Protocol.replyHdr); os.writeInt(_current.requestId); os.writeByte(ok ? ReplyStatus.replyOK : ReplyStatus.replyUserException); diff --git a/java/test/src/main/java/test/Ice/interceptor/Client.java b/java/test/src/main/java/test/Ice/interceptor/Client.java index dafae39f792..bbb62f152b9 100644 --- a/java/test/src/main/java/test/Ice/interceptor/Client.java +++ b/java/test/src/main/java/test/Ice/interceptor/Client.java @@ -111,6 +111,16 @@ public class Client extends test.TestHelper test(prx.amdAddWithRetry(33, 12) == 45); test(interceptor.getLastOperation().equals("amdAddWithRetry")); test(interceptor.getLastStatus()); + { + java.util.Map ctx = new java.util.HashMap<>(); + ctx.put("retry", "yes"); + for(int i = 0; i < 10; ++i) + { + test(prx.amdAdd(33, 12, ctx) == 45); + test(interceptor.getLastOperation().equals("amdAdd")); + test(interceptor.getLastStatus()); + } + } out.println("ok"); out.print("testing user exception... "); out.flush(); diff --git a/java/test/src/main/java/test/Ice/interceptor/InterceptorI.java b/java/test/src/main/java/test/Ice/interceptor/InterceptorI.java index 5ea248e1c43..afd5597f6b0 100644 --- a/java/test/src/main/java/test/Ice/interceptor/InterceptorI.java +++ b/java/test/src/main/java/test/Ice/interceptor/InterceptorI.java @@ -70,6 +70,15 @@ class InterceptorI extends com.zeroc.Ice.DispatchInterceptor current.ctx.put("retry", "no"); } + else if(current.ctx.get("retry") != null && current.ctx.get("retry").equals("yes")) + { + // + // Retry the dispatch to ensure that abandoning the result of the dispatch + // works fine and is thread-safe + // + _servant.ice_dispatch(request); + _servant.ice_dispatch(request); + } CompletionStage f = _servant.ice_dispatch(request); _lastStatus = f != null; -- cgit v1.2.3