Fixed bug 2972 & 3013

4 files changed, 93 insertions, 5 deletions

diff --git a/cpp/include/IceUtil/Timer.h b/cpp/include/IceUtil/Timer.h
index 545fecdddfe..f0920bb03b3 100644
--- a/cpp/include/IceUtil/Timer.h
+++ b/cpp/include/IceUtil/Timer.h
@@ -53,7 +53,8 @@ public:
     Timer();
 
     //
-    // Destroy the timer and join with its execution thread.
+    // Destroy the timer and detach its execution thread if the calling thread 
+    // is the timer thread, join the timer execution thread otherwise.
     //
     void destroy();
 
diff --git a/cpp/src/Ice/ProxyFactory.cpp b/cpp/src/Ice/ProxyFactory.cpp
index 48e9460bada..2fc5316eb6d 100644
--- a/cpp/src/Ice/ProxyFactory.cpp
+++ b/cpp/src/Ice/ProxyFactory.cpp
@@ -248,7 +248,14 @@ IceInternal::ProxyFactory::checkRetryAfterException(const LocalException& ex,
     {
         if(out)
         {
-            _instance->timer()->schedule(new RetryTask(out), IceUtil::Time::milliSeconds(interval));
+            try
+            {
+                _instance->timer()->schedule(new RetryTask(out), IceUtil::Time::milliSeconds(interval));
+            }
+            catch(const IceUtil::IllegalArgumentException&) // Expected if the communicator destroyed the timer.
+            {
+                throw CommunicatorDestroyedException(__FILE__, __LINE__); 
+            }
         }
         else
         {
diff --git a/cpp/src/IceUtil/Timer.cpp b/cpp/src/IceUtil/Timer.cpp
index 8a70432709d..a5b2175c425 100644
--- a/cpp/src/IceUtil/Timer.cpp
+++ b/cpp/src/IceUtil/Timer.cpp
@@ -32,7 +32,15 @@ Timer::destroy()
         _tasks.clear();
         _tokens.clear();
     }
-    getThreadControl().join();
+
+    if(getThreadControl() == ThreadControl())
+    {
+        getThreadControl().detach();
+    }
+    else
+    {
+        getThreadControl().join();
+    }
 }
 
 void
@@ -41,7 +49,7 @@ Timer::schedule(const TimerTaskPtr& task, const IceUtil::Time& delay)
     IceUtil::Monitor<IceUtil::Mutex>::Lock sync(_monitor);
     if(_destroyed)
     {
-        return;
+        throw IllegalArgumentException(__FILE__, __LINE__, "timer destroyed");
     }
 
     IceUtil::Time time = IceUtil::Time::now(IceUtil::Time::Monotonic) + delay;
@@ -64,7 +72,7 @@ Timer::scheduleRepeated(const TimerTaskPtr& task, const IceUtil::Time& delay)
     IceUtil::Monitor<IceUtil::Mutex>::Lock sync(_monitor);
     if(_destroyed)
     {
-        return;
+        throw IllegalArgumentException(__FILE__, __LINE__, "timer destroyed");
     }
 
     const Token token(IceUtil::Time::now(IceUtil::Time::Monotonic) + delay, delay, task);
diff --git a/cpp/test/IceUtil/timer/Client.cpp b/cpp/test/IceUtil/timer/Client.cpp
index 1af264b6120..49d6947869d 100644
--- a/cpp/test/IceUtil/timer/Client.cpp
+++ b/cpp/test/IceUtil/timer/Client.cpp
@@ -98,6 +98,44 @@ private:
 };
 typedef IceUtil::Handle<TestTask> TestTaskPtr;
 
+
+class DestroyTask : public IceUtil::TimerTask, IceUtil::Monitor<IceUtil::Mutex>
+{
+public:
+
+    DestroyTask(const IceUtil::TimerPtr& timer) : _timer(timer), _run(false)
+    {
+    }
+
+    virtual void 
+    runTimerTask()
+    {
+        Lock sync(*this);
+        _timer->destroy();
+        _run = true;
+        notify();
+    }
+
+    virtual void
+    waitForRun()
+    {
+        Lock sync(*this);
+        while(!_run)
+        {
+            if(!timedWait(IceUtil::Time::seconds(10)))
+            {
+                test(false); // Timeout.
+            }
+        }
+    }
+
+private:
+
+    IceUtil::TimerPtr _timer;
+    bool _run;
+};
+typedef IceUtil::Handle<DestroyTask> DestroyTaskPtr;
+
 int main(int argc, char* argv[])
 {
     cout << "testing timer... " << flush;
@@ -183,5 +221,39 @@ int main(int argc, char* argv[])
         timer->destroy();
     }
     cout << "ok" << endl;
+
+    cout << "testing timer destroy... " << flush;
+    {
+        {
+            IceUtil::TimerPtr timer = new IceUtil::Timer();
+            DestroyTaskPtr destroyTask = new DestroyTask(timer);
+            timer->schedule(destroyTask, IceUtil::Time());
+            destroyTask->waitForRun();
+            try
+            {
+                timer->schedule(destroyTask, IceUtil::Time());
+            }
+            catch(const IceUtil::IllegalArgumentException&)
+            {
+                // Expected;
+            }
+        }
+        {
+            IceUtil::TimerPtr timer = new IceUtil::Timer();
+            TestTaskPtr testTask = new TestTask();
+            timer->schedule(testTask, IceUtil::Time());
+            timer->destroy();
+            try
+            {
+                timer->schedule(testTask, IceUtil::Time());
+            }
+            catch(const IceUtil::IllegalArgumentException&)
+            {
+                // Expected;
+            }
+        }
+    }
+    cout << "ok" << endl;
+
     return EXIT_SUCCESS;
 }