Fixed thread idle time bug (#2537)

This PR fixes a bug introduced with the C# thread pool back pressure fix related to the server/thread idle time. It also adds tests for C++/Java.

6 files changed, 121 insertions, 40 deletions

diff --git a/cpp/test/Ice/adapterDeactivation/AllTests.cpp b/cpp/test/Ice/adapterDeactivation/AllTests.cpp
index 1a7a46da718..3ff45871b37 100644
--- a/cpp/test/Ice/adapterDeactivation/AllTests.cpp
+++ b/cpp/test/Ice/adapterDeactivation/AllTests.cpp
@@ -6,6 +6,10 @@
 #include <TestHelper.h>
 #include <Test.h>
 
+#ifdef ICE_CPP11_MAPPING
+#   include <thread>
+#endif
+
 using namespace std;
 using namespace Ice;
 using namespace Test;
@@ -210,23 +214,47 @@ allTests(Test::TestHelper* helper)
         cout << "ok" << endl;
     }
 
+#ifdef ICE_CPP11_MAPPING
     cout << "testing server idle time..." << flush;
-    {
-        InitializationData idleInitData;
-        idleInitData.properties = communicator->getProperties()->clone();
-        idleInitData.properties->setProperty("Ice.ServerIdleTime", "1");
-    #ifdef _WIN32
-        // With our Windows implementation, the thread pool threads have to be idle first before server idle time is
-        // checked
-        idleInitData.properties->setProperty("Ice.ThreadPool.Server.ThreadIdleTime", "1");
-    #endif
-        CommunicatorHolder idleCommunicator(idleInitData);
-        // The server thread pool is started lazily so we need to create an object adapter and activate it.
-        ObjectAdapterPtr idleOA = idleCommunicator->createObjectAdapterWithEndpoints("IdleOA", "tcp -h 127.0.0.1");
-        idleOA->activate();
-        idleCommunicator->waitForShutdown();
-    }
+    std::thread thread1([]()
+        {
+            InitializationData idleInitData;
+            idleInitData.properties = createProperties();
+            idleInitData.properties->setProperty("Ice.ServerIdleTime", "1");
+        #ifdef _WIN32
+            // With our Windows implementation, the thread pool threads have to be idle first before server idle time is
+            // checked
+            idleInitData.properties->setProperty("Ice.ThreadPool.Server.ThreadIdleTime", "1");
+        #endif
+            CommunicatorHolder idleCommunicator(idleInitData);
+            // The server thread pool is started lazily so we need to create an object adapter and activate it.
+            ObjectAdapterPtr idleOA = idleCommunicator->createObjectAdapterWithEndpoints("IdleOA", "tcp -h 127.0.0.1");
+            idleOA->activate();
+            idleCommunicator->waitForShutdown();
+            idleCommunicator->destroy();
+        });
+    std::thread thread2([]()
+        {
+            InitializationData idleInitData;
+            idleInitData.properties = createProperties();
+            idleInitData.properties->setProperty("Ice.ServerIdleTime", "0");
+        #ifdef _WIN32
+            // With our Windows implementation, the thread pool threads have to be idle first before server idle time is
+            // checked
+            idleInitData.properties->setProperty("Ice.ThreadPool.Server.ThreadIdleTime", "1");
+        #endif
+            CommunicatorHolder idleCommunicator(idleInitData);
+            // The server thread pool is started lazily so we need to create an object adapter and activate it.
+            ObjectAdapterPtr idleOA = idleCommunicator->createObjectAdapterWithEndpoints("IdleOA", "tcp -h 127.0.0.1");
+            idleOA->activate();
+            std::this_thread::sleep_for(1200ms);
+            test(!idleCommunicator->isShutdown());
+            idleCommunicator->destroy();
+        });
+    thread1.join();
+    thread2.join();
     cout << "ok" << endl;
+#endif
 
     return obj;
 }
diff --git a/csharp/src/Ice/ThreadPool.cs b/csharp/src/Ice/ThreadPool.cs
index 467c5bdfdbf..b813849acf7 100644
--- a/csharp/src/Ice/ThreadPool.cs
+++ b/csharp/src/Ice/ThreadPool.cs
@@ -163,7 +163,7 @@ namespace IceInternal
             _threadIndex = 0;
             _inUse = 0;
             _serialize = properties.getPropertyAsInt(_prefix + ".Serialize") > 0;
-            _serverIdleTime = timeout;
+            _serverIdleTime = timeout <= 0 ? Timeout.InfiniteTimeSpan : TimeSpan.FromSeconds(timeout);
 
             string programName = properties.getProperty("Ice.ProgramName");
             if(programName.Length > 0)
@@ -222,7 +222,7 @@ namespace IceInternal
             _size = size;
             _sizeMax = sizeMax;
             _sizeWarn = sizeWarn;
-            _threadIdleTime = threadIdleTime;
+            _threadIdleTime = threadIdleTime <= 0 ? Timeout.InfiniteTimeSpan : TimeSpan.FromSeconds(threadIdleTime);
 
             int stackSize = properties.getPropertyAsInt(_prefix + ".StackSize");
             if(stackSize < 0)
@@ -500,9 +500,9 @@ namespace IceInternal
                             return;
                         }
 
-                        if(_threadIdleTime > 0)
+                        if(_threadIdleTime != Timeout.InfiniteTimeSpan)
                         {
-                            if(!Monitor.Wait(this, _threadIdleTime * 1000) && _workItems.Count == 0) // If timeout
+                            if(!Monitor.Wait(this, _threadIdleTime) && _workItems.Count == 0) // If timeout
                             {
                                 if(_destroyed)
                                 {
@@ -538,7 +538,7 @@ namespace IceInternal
                                 }
 
                                 Debug.Assert(_threads.Count == 1);
-                                if(!Monitor.Wait(this, _serverIdleTime * 1000) && !_destroyed)
+                                if(!Monitor.Wait(this, _serverIdleTime) && !_destroyed)
                                 {
                                     _workItems.Enqueue(c =>
                                         {
@@ -856,8 +856,8 @@ namespace IceInternal
         private readonly int _sizeWarn; // If _inUse reaches _sizeWarn, a "low on threads" warning will be printed.
         private readonly bool _serialize; // True if requests need to be serialized over the connection.
         private readonly ThreadPriority _priority;
-        private readonly int _serverIdleTime;
-        private readonly int _threadIdleTime;
+        private readonly TimeSpan _serverIdleTime;
+        private readonly TimeSpan _threadIdleTime;
         private readonly int _stackSize;
 
         private List<WorkerThread> _threads; // All threads, running or not.
diff --git a/csharp/test/Ice/adapterDeactivation/AllTests.cs b/csharp/test/Ice/adapterDeactivation/AllTests.cs
index b6a3a63cc0b..86a4ca49646 100644
--- a/csharp/test/Ice/adapterDeactivation/AllTests.cs
+++ b/csharp/test/Ice/adapterDeactivation/AllTests.cs
@@ -3,6 +3,7 @@
 //
 
 using System;
+using System.Threading.Tasks;
 
 namespace Ice
 {
@@ -10,7 +11,7 @@ namespace Ice
     {
         public class AllTests : global::Test.AllTests
         {
-            public static Test.TestIntfPrx allTests(global::Test.TestHelper helper)
+            public static async Task<Test.TestIntfPrx> allTests(global::Test.TestHelper helper)
             {
                 Ice.Communicator communicator = helper.communicator();
                 var output = helper.getWriter();
@@ -203,22 +204,42 @@ namespace Ice
 
                 output.Write("testing server idle time...");
                 output.Flush();
-                {
-                    Ice.InitializationData initData = new Ice.InitializationData()
+                Task task1 = Task.Run(() =>
                     {
-                        properties = communicator.getProperties().ice_clone_(),
-                    };
-                    initData.properties.setProperty("Ice.ServerIdleTime", "1");
-                    // The thread pool threads have to be idle first before server idle time is checked.
-                    initData.properties.setProperty("Ice.ThreadPool.Server.ThreadIdleTime", "1");
-                    using(Ice.Communicator idleCommunicator = Ice.Util.initialize(initData))
+                        Ice.InitializationData initData = new Ice.InitializationData()
+                        {
+                            properties = Ice.Util.createProperties(),
+                        };
+                        initData.properties.setProperty("Ice.ServerIdleTime", "1");
+                        // The thread pool threads have to be idle first before server idle time is checked.
+                        initData.properties.setProperty("Ice.ThreadPool.Server.ThreadIdleTime", "1");
+                        using(Ice.Communicator idleCommunicator = Ice.Util.initialize(initData))
+                        {
+                            ObjectAdapter idleOA = idleCommunicator.createObjectAdapterWithEndpoints("IdleAdapter", "tcp -h 127.0.0.1");
+                            idleOA.activate();
+                            idleCommunicator.waitForShutdown();
+                        }
+                    });
+                Task task2 = Task.Run(async () =>
                     {
-                        ObjectAdapter idleOA = idleCommunicator.createObjectAdapterWithEndpoints("IdleAdapter", "tcp -h 127.0.0.1 ");
-                        idleOA.activate();
-                        idleCommunicator.waitForShutdown();
-                    }
-                 }
-                 output.WriteLine("ok");
+                        Ice.InitializationData initData = new Ice.InitializationData()
+                        {
+                            properties = Ice.Util.createProperties(),
+                        };
+                        initData.properties.setProperty("Ice.ServerIdleTime", "0");
+                        // The thread pool threads have to be idle first before server idle time is checked.
+                        initData.properties.setProperty("Ice.ThreadPool.Server.ThreadIdleTime", "1");
+                        using(Ice.Communicator idleCommunicator = Ice.Util.initialize(initData))
+                        {
+                            ObjectAdapter idleOA = idleCommunicator.createObjectAdapterWithEndpoints("IdleAdapter", "tcp -h 127.0.0.1");
+                            idleOA.activate();
+                            await Task.Delay(1100);
+                            test(!idleCommunicator.isShutdown());
+                        }
+                    });
+                await task1;
+                await task2;
+                output.WriteLine("ok");
 
                 return obj;
             }
diff --git a/csharp/test/Ice/adapterDeactivation/Client.cs b/csharp/test/Ice/adapterDeactivation/Client.cs
index ddf0a4b62da..04266bd78fe 100644
--- a/csharp/test/Ice/adapterDeactivation/Client.cs
+++ b/csharp/test/Ice/adapterDeactivation/Client.cs
@@ -14,7 +14,7 @@ namespace Ice
             {
                 using(var communicator = initialize(ref args))
                 {
-                    AllTests.allTests(this);
+                    AllTests.allTests(this).Wait();
                 }
             }
 
diff --git a/csharp/test/Ice/adapterDeactivation/Collocated.cs b/csharp/test/Ice/adapterDeactivation/Collocated.cs
index eb75656e09d..66423182d32 100644
--- a/csharp/test/Ice/adapterDeactivation/Collocated.cs
+++ b/csharp/test/Ice/adapterDeactivation/Collocated.cs
@@ -25,7 +25,7 @@ namespace Ice
                     var locator = new ServantLocatorI();
                     adapter.addServantLocator(locator, "");
 
-                    AllTests.allTests(this);
+                    AllTests.allTests(this).Wait();
 
                     adapter.waitForDeactivate();
                 }
diff --git a/java/test/src/main/java/test/Ice/adapterDeactivation/AllTests.java b/java/test/src/main/java/test/Ice/adapterDeactivation/AllTests.java
index 709864dabf5..ea2e2c39e18 100644
--- a/java/test/src/main/java/test/Ice/adapterDeactivation/AllTests.java
+++ b/java/test/src/main/java/test/Ice/adapterDeactivation/AllTests.java
@@ -211,9 +211,10 @@ public class AllTests
 
         out.print("testing server idle time...");
         out.flush();
+        Thread thread1 = new Thread(() ->
         {
             com.zeroc.Ice.InitializationData initData = new com.zeroc.Ice.InitializationData();
-            initData.properties = communicator.getProperties()._clone();
+            initData.properties = com.zeroc.Ice.Util.createProperties();
             initData.properties.setProperty("Ice.ServerIdleTime", "1");
             try (com.zeroc.Ice.Communicator idleCommunicator = com.zeroc.Ice.Util.initialize(initData)) {
                 com.zeroc.Ice.ObjectAdapter adapter =
@@ -221,6 +222,37 @@ public class AllTests
                 adapter.activate();
                 idleCommunicator.waitForShutdown();
             }
+        });
+        Thread thread2 = new Thread(() ->
+        {
+            com.zeroc.Ice.InitializationData initData = new com.zeroc.Ice.InitializationData();
+            initData.properties = com.zeroc.Ice.Util.createProperties();
+            initData.properties.setProperty("Ice.ServerIdleTime", "1");
+            try (com.zeroc.Ice.Communicator idleCommunicator = com.zeroc.Ice.Util.initialize(initData)) {
+                com.zeroc.Ice.ObjectAdapter adapter =
+                    idleCommunicator.createObjectAdapterWithEndpoints("IdleAdapter", "tcp -h 127.0.0.1");
+                adapter.activate();
+                try
+                {
+                    Thread.sleep(1200);
+                }
+                catch(InterruptedException ex)
+                {
+                    test(false);
+                }
+                test(idleCommunicator.isShutdown());
+            }
+        });
+        thread1.start();
+        thread2.start();
+        try
+        {
+            thread1.join();
+            thread2.join();
+        }
+        catch (InterruptedException ex)
+        {
+            test(false);
         }
         out.println("ok");