addressed few more concerns.

6 files changed, 86 insertions, 37 deletions

diff --git a/cpp/demo/Ice/session/ReapThread.h b/cpp/demo/Ice/session/ReapThread.h
new file mode 100755
index 00000000000..7667e8b16dd
--- /dev/null
+++ b/cpp/demo/Ice/session/ReapThread.h
@@ -0,0 +1,59 @@
+// **********************************************************************
+//
+// Copyright (c) 2003-2005 ZeroC, Inc. All rights reserved.
+//
+// This copy of Ice is licensed to you under the terms described in the
+// ICE_LICENSE file included in this distribution.
+//
+// **********************************************************************
+
+#ifndef REAP_THREAD_H
+#define REAP_THREAD_H
+
+#include <IceUtil/Thread.h>
+#include <IceUtil/StaticMutex.h>
+#include <SessionI.h>
+
+class ReapThread;
+typedef IceUtil::Handle<ReapThread> ReapThreadPtr;
+class ReapThread : public IceUtil::Thread, public IceUtil::Monitor<IceUtil::Mutex>
+{
+public:
+
+    static ReapThreadPtr& instance();
+
+    virtual ~ReapThread();
+
+    virtual void run();
+
+    // XXX Rename to destroy().
+    //
+    // I named it terminate because destroy() methods in Java result
+    // in a deprecation warning. If you want to use destroy() in Java
+    // then I would have to use a runnable which means the demo isn't
+    // the same code. Given the difference is a method name, I didn't
+    // think it was worth the cost of different code.
+    //
+    void terminate();
+
+    // XXX: The alternative here is to make timestamp() a slice
+    // method. However, this means that we're adding methods to the
+    // slice interface which is only required for the reaping thread,
+    // and it obviously couldn't return an IceUtil::Time but instead
+    // some other representation...
+    //
+    void add(const ::Demo::SessionPrx&, const SessionIPtr&);
+
+private:
+
+    ReapThread();
+
+    const IceUtil::Time _timeout;
+    bool _terminated;
+    std::map< ::Demo::SessionPrx, SessionIPtr> _sessions;
+
+    static ReapThreadPtr _instance;
+    static IceUtil::StaticMutex _instanceMutex;
+};
+
+#endif
diff --git a/cpp/demo/Ice/session/Server.cpp b/cpp/demo/Ice/session/Server.cpp
index d9551e7ddfc..3c5da0ef43a 100644
--- a/cpp/demo/Ice/session/Server.cpp
+++ b/cpp/demo/Ice/session/Server.cpp
@@ -18,8 +18,8 @@ int
 run(int argc, char* argv[], const Ice::CommunicatorPtr& communicator)
 {
     Ice::ObjectAdapterPtr adapter = communicator->createObjectAdapter("SessionFactory");
-    ReapThreadPtr reaper = new ReapThread(IceUtil::Time::seconds(10));
-    adapter->add(new SessionFactoryI(reaper), Ice::stringToIdentity("SessionFactory"));
+    ReapThreadPtr reaper = ReapThread::instance();
+    adapter->add(new SessionFactoryI, Ice::stringToIdentity("SessionFactory"));
     adapter->activate();
     reaper->start();
     communicator->waitForShutdown();
diff --git a/cpp/demo/Ice/session/SessionFactoryI.cpp b/cpp/demo/Ice/session/SessionFactoryI.cpp
index 72fe47e5af1..66d27f083e8 100755
--- a/cpp/demo/Ice/session/SessionFactoryI.cpp
+++ b/cpp/demo/Ice/session/SessionFactoryI.cpp
@@ -9,12 +9,12 @@
 
 #include <Ice/Ice.h>
 #include <SessionFactoryI.h>
+#include <ReapThread.h>
 
 using namespace std;
 using namespace Demo;
 
-SessionFactoryI::SessionFactoryI(const ReapThreadPtr& reapThread) :
-    _reapThread(reapThread)
+SessionFactoryI::SessionFactoryI()
 {
 }
 
@@ -29,7 +29,7 @@ SessionFactoryI::create(const Ice::Current& c)
 
     SessionIPtr session = new SessionI;
     SessionPrx proxy = SessionPrx::uncheckedCast(c.adapter->addWithUUID(session));
-    _reapThread->add(proxy, session);
+    ReapThread::instance()->add(proxy, session);
     return proxy;
 }
 
diff --git a/cpp/demo/Ice/session/SessionFactoryI.h b/cpp/demo/Ice/session/SessionFactoryI.h
index fff5ea24578..6987d7f1d22 100755
--- a/cpp/demo/Ice/session/SessionFactoryI.h
+++ b/cpp/demo/Ice/session/SessionFactoryI.h
@@ -11,34 +11,16 @@
 #define SESSION_FACTORY_I_H
 
 #include <Session.h>
-#include <ReapThread.h> // XXX Not checked in.
 
-class SessionFactoryI : public ::Demo::SessionFactory, public IceUtil::Mutex
+class SessionFactoryI : public Demo::SessionFactory, public IceUtil::Mutex
 {
 public:
 
-    SessionFactoryI(const ReapThreadPtr&);
+    SessionFactoryI();
     virtual ~SessionFactoryI();
 
-    virtual ::Demo::SessionPrx create(const ::Ice::Current&);
+    virtual Demo::SessionPrx create(const ::Ice::Current&);
     virtual void shutdown(const Ice::Current&);
-
-private:
-
-    // XXX Why does the factory have to know the reaper thread? The
-    // sessions should know, they can register themselves directly
-    // with the reaper thread.
-    //
-    // If I do this it means that either the reap thread must be a
-    // singleton, or I need to pass the reap thread to session and
-    // tell the session its proxy which it currently does not
-    // know. Since the session factory knows both, this seems like a
-    // better solution.
-    //
-    // XXX Make it a singleton, because it is a singleton. It doesn't
-    // belong here.
-    //
-    const ReapThreadPtr _reapThread;
 };
 
 #endif
diff --git a/cpp/demo/Ice/session/SessionI.cpp b/cpp/demo/Ice/session/SessionI.cpp
index e3dc2c4937d..4cf07aa8fa3 100755
--- a/cpp/demo/Ice/session/SessionI.cpp
+++ b/cpp/demo/Ice/session/SessionI.cpp
@@ -9,6 +9,7 @@
 
 #include <Ice/Ice.h>
 #include <SessionFactoryI.h>
+#include <SessionI.h>
 
 using namespace std;
 using namespace Demo;
@@ -42,7 +43,11 @@ HelloPrx
 SessionI::createHello(const Ice::Current& c)
 {
     Lock sync(*this);
-    // XXX Check for destruction missing.
+    if(_destroy)
+    {
+	throw Ice::ObjectNotExistException(__FILE__, __LINE__);
+    }
+
     HelloPrx hello = HelloPrx::uncheckedCast(c.adapter->addWithUUID(new HelloI(_nextId++)));
     _objs.push_back(hello);
     return hello;
@@ -52,7 +57,11 @@ void
 SessionI::refresh(const Ice::Current& c)
 {
     Lock sync(*this);
-    // XXX Check for destruction missing.
+    if(_destroy)
+    {
+	throw Ice::ObjectNotExistException(__FILE__, __LINE__);
+    }
+
     _timestamp = IceUtil::Time::now();
 }
 
@@ -60,7 +69,11 @@ void
 SessionI::destroy(const Ice::Current& c)
 {
     Lock sync(*this);
-    // XXX Check for destruction missing.
+    if(_destroy)
+    {
+	throw Ice::ObjectNotExistException(__FILE__, __LINE__);
+    }
+
     _destroy = true;
 
     cout << "The session #" << Ice::identityToString(c.id) << " is now destroyed." << endl;
diff --git a/cpp/demo/Ice/session/SessionI.h b/cpp/demo/Ice/session/SessionI.h
index df2321460b8..7988e46078d 100755
--- a/cpp/demo/Ice/session/SessionI.h
+++ b/cpp/demo/Ice/session/SessionI.h
@@ -13,16 +13,11 @@
 #include <Session.h>
 #include <list>
 
-// XXX Get rid of leading ::, i.e., use Demo::, not ::Demo::
-// (everywhere).
-// Style: The other demos all use ::Demo::
-// XXX All other demos should be changed then. We don't use leading :: if they are not necessary.
-
-class SessionI : public ::Demo::Session, public IceUtil::Mutex
+class SessionI : public Demo::Session, public IceUtil::Mutex
 {
 public:
 
-    virtual ::Demo::HelloPrx createHello(const Ice::Current&);
+    virtual Demo::HelloPrx createHello(const Ice::Current&);
     virtual void refresh(const Ice::Current&);
     virtual void destroy(const Ice::Current&);
 
@@ -39,7 +34,7 @@ private:
     IceUtil::Time _timestamp; // The last time the session was refreshed.
     
     int _nextId; // The id of the next hello object. This is used for tracing purposes.
-    std::list< ::Demo::HelloPrx> _objs; // List of per-client allocated Hello objects.
+    std::list< Demo::HelloPrx> _objs; // List of per-client allocated Hello objects.
     bool _destroy;
 };