fixes

3 files changed, 16 insertions, 5 deletions

diff --git a/cpp/demo/Ice/session/Session.ice b/cpp/demo/Ice/session/Session.ice
index 0a687ae651d..9dca4b31fe0 100644
--- a/cpp/demo/Ice/session/Session.ice
+++ b/cpp/demo/Ice/session/Session.ice
@@ -13,38 +13,46 @@
 module Demo
 {
 
+/** XXX Comment missing XXX **/
+
 interface Session
 {
     /**
-     * Refresh a session. If a session is not refreshed on a regular
-     * basis by the client it will be automatically destroyed.
+     *
+     * Refresh this session. If a session is not refreshed on a
+     * regular basis by the client it will be automatically destroyed.
      *
      **/
     void refresh();
 
     /**
-     * Callback that the session has been destroyed.
+     *
+     * Callback that the session has been destroyed. XXX I don't understand this method. Who calls this? Isn't this server-side only? If so, it doesn't belong into the contract. XXX
      *
      **/
     void destroyed();
 
     /**
-     * Destroy a session.
+     *
+     * Destroy this session.
      *
      **/
     void destroy();
 };
 
+/** XXX Comment missing. Why "SessionManager"? From the contract perspective, this is a SessionFactory. XXX **/
 interface SessionManager
 {
     /**
+     *
      * Create a new session.
      *
      **/
     Session* create();
 
     /**
-     * Shutdown the server
+     *
+     * Shutdown the server.
      *
      **/
     void shutdown();
diff --git a/cpp/demo/Ice/session/SessionManagerI.cpp b/cpp/demo/Ice/session/SessionManagerI.cpp
index 5e761a8aa86..ecbc37f9aa2 100755
--- a/cpp/demo/Ice/session/SessionManagerI.cpp
+++ b/cpp/demo/Ice/session/SessionManagerI.cpp
@@ -13,6 +13,8 @@
 using namespace std;
 using namespace Demo;
 
+// XXX Cyclic dependency between ReapThread and SessionManagerI.cpp!! Set to 0 in both destroy() functions. XXX
+
 ReapThread::ReapThread(const SessionManagerIPtr& manager, const IceUtil::Time& timeout) :
     _destroy(false),
     _timeout(timeout),
diff --git a/cpp/demo/Ice/session/SessionManagerI.h b/cpp/demo/Ice/session/SessionManagerI.h
index 4c815fa6b45..26712dd414b 100755
--- a/cpp/demo/Ice/session/SessionManagerI.h
+++ b/cpp/demo/Ice/session/SessionManagerI.h
@@ -50,6 +50,7 @@ public:
     virtual Demo::SessionPrx create(const Ice::Current&) = 0;
     virtual void shutdown(const Ice::Current&);
 
+// XXX Why are these not protected? XXX Get rid off add(), see comment above. XXX Get rid of remove(), too! This is evil, session are not supposed to remove themselves, this only leads to deadlocks. They must get reaped. XXX Get rid of refresh() - why is this needed? XXX
     void destroy();
     void add(const Demo::SessionPrx&);
     void remove(const Ice::Identity&);