summaryrefslogtreecommitdiff
path: root/cpp
diff options
context:
space:
mode:
Diffstat (limited to 'cpp')
-rwxr-xr-xcpp/demo/Ice/session/Client.cpp19
-rwxr-xr-xcpp/demo/Ice/session/ReapThread.cpp19
-rwxr-xr-xcpp/demo/Ice/session/ReapThread.h23
-rw-r--r--cpp/demo/Ice/session/Server.cpp6
-rw-r--r--cpp/demo/Ice/session/Session.ice20
-rwxr-xr-xcpp/demo/Ice/session/SessionFactoryI.cpp6
-rwxr-xr-xcpp/demo/Ice/session/SessionFactoryI.h6
-rwxr-xr-xcpp/demo/Ice/session/SessionI.cpp5
-rwxr-xr-xcpp/demo/Ice/session/SessionI.h8
9 files changed, 68 insertions, 44 deletions
diff --git a/cpp/demo/Ice/session/Client.cpp b/cpp/demo/Ice/session/Client.cpp
index 64022d4fbd9..84bc1d15e97 100755
--- a/cpp/demo/Ice/session/Client.cpp
+++ b/cpp/demo/Ice/session/Client.cpp
@@ -9,7 +9,7 @@
#include <Ice/Ice.h>
#include <IceUtil/Thread.h>
-#include <Session.h>
+#include <Session.h>
using namespace std;
using namespace Demo;
@@ -64,6 +64,7 @@ private:
const IceUtil::Time _timeout;
bool _terminated;
};
+
typedef IceUtil::Handle<SessionRefreshThread> SessionRefreshThreadPtr;
void
@@ -71,9 +72,9 @@ menu()
{
cout <<
"usage:\n"
- "c: create new hello\n"
- "0-9: greet hello object\n"
- "s: shutdown server\n"
+ "c: create a new per-client hello object\n"
+ "0-9: send a greeting to a hello object\n"
+ "s: shutdown the server\n"
"x: exit\n"
"t: exit without destroying the session\n"
"?: help\n";
@@ -132,14 +133,14 @@ run(int argc, char* argv[], const Ice::CommunicatorPtr& communicator)
}
else
{
- cout << "index is too high. " << hellos.size() << " exist so far. "
- << "Use 'c' to create a new hello object." << endl;
+ cout << "Index is too high. " << hellos.size() << " hello objects exist so far.\n"
+ << "Use `c' to create a new hello object." << endl;
}
}
else if(c == 'c')
{
hellos.push_back(session->createHello());
- cout << "created hello object " << hellos.size()-1 << endl;
+ cout << "Created hello object #" << hellos.size() - 1 << endl;
}
else if(c == 's')
{
@@ -160,7 +161,7 @@ run(int argc, char* argv[], const Ice::CommunicatorPtr& communicator)
}
else
{
- cout << "unknown command `" << c << "'" << endl;
+ cout << "Unknown command `" << c << "'." << endl;
menu();
}
}
@@ -181,6 +182,8 @@ run(int argc, char* argv[], const Ice::CommunicatorPtr& communicator)
return EXIT_SUCCESS;
}
+// XXX Rewrite to use Application. See ../callback for an example.
+
int
main(int argc, char* argv[])
{
diff --git a/cpp/demo/Ice/session/ReapThread.cpp b/cpp/demo/Ice/session/ReapThread.cpp
index 1cd8bae6f8b..9380aee84fa 100755
--- a/cpp/demo/Ice/session/ReapThread.cpp
+++ b/cpp/demo/Ice/session/ReapThread.cpp
@@ -20,6 +20,7 @@ ReapThreadPtr&
ReapThread::instance()
{
IceUtil::StaticMutex::Lock sync(_instanceMutex);
+
if(!_instance)
{
_instance = new ReapThread;
@@ -36,11 +37,16 @@ void
ReapThread::run()
{
Lock sync(*this);
+
while(!_terminated)
{
timedWait(_timeout);
+
if(!_terminated)
{
+ // XXX Session destruction may take time in a real-world
+ // example. Therefore now should be computed in the loop
+ // for each iteration.
IceUtil::Time now = IceUtil::Time::now();
map<SessionPrx, SessionIPtr>::iterator p = _sessions.begin();
while(p != _sessions.end())
@@ -52,6 +58,7 @@ ReapThread::run()
cout << "The session #" << Ice::identityToString(p->first->ice_getIdentity())
<< " has timed out." << endl;
p->first->destroy();
+ // XXX This can be simplified to _sessions.erase(p++);
map<SessionPrx, SessionIPtr>::iterator tmp = p;
++p;
_sessions.erase(tmp);
@@ -63,6 +70,7 @@ ReapThread::run()
}
catch(const Ice::ObjectNotExistException&)
{
+ // XXX This can be simplified to _sessions.erase(p++);
map<SessionPrx, SessionIPtr>::iterator tmp = p;
++p;
_sessions.erase(tmp);
@@ -76,12 +84,11 @@ void
ReapThread::terminate()
{
Lock sync(*this);
+
_terminated = true;
notify();
- for(map<SessionPrx, SessionIPtr>::const_iterator p = _sessions.begin();
- p != _sessions.end();
- ++p)
+ for(map<SessionPrx, SessionIPtr>::const_iterator p = _sessions.begin(); p != _sessions.end(); ++p)
{
try
{
@@ -89,9 +96,10 @@ ReapThread::terminate()
}
catch(const Ice::Exception&)
{
- // Ignore the exception
+ // Ignore.
}
}
+
_sessions.clear();
}
@@ -99,6 +107,9 @@ void
ReapThread::add(const SessionPrx& proxy, const SessionIPtr& session)
{
Lock sync(*this);
+ // XXX Don't use make_pair, it's not portable. We had to remove it
+ // from most of our code, because the Sun compiler and others had
+ // problems with it.y
_sessions.insert(make_pair(proxy, session));
}
diff --git a/cpp/demo/Ice/session/ReapThread.h b/cpp/demo/Ice/session/ReapThread.h
index 7667e8b16dd..012dac82f47 100755
--- a/cpp/demo/Ice/session/ReapThread.h
+++ b/cpp/demo/Ice/session/ReapThread.h
@@ -22,27 +22,13 @@ public:
static ReapThreadPtr& instance();
- virtual ~ReapThread();
+ virtual ~ReapThread(); // XXX Destructor does nothing, get rid of it.
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&);
+ void add(const Demo::SessionPrx&, const SessionIPtr&);
private:
@@ -50,7 +36,10 @@ private:
const IceUtil::Time _timeout;
bool _terminated;
- std::map< ::Demo::SessionPrx, SessionIPtr> _sessions;
+ // XXX Why is this a map and not simply a list of
+ // pair<Demo::SessionPrx, SessionIPtr> (or a list of structs with
+ // these elements)? The sorting of a map is needed nowhere.
+ std::map< Demo::SessionPrx, SessionIPtr> _sessions;
static ReapThreadPtr _instance;
static IceUtil::StaticMutex _instanceMutex;
diff --git a/cpp/demo/Ice/session/Server.cpp b/cpp/demo/Ice/session/Server.cpp
index 3c5da0ef43a..6199c3d5025 100644
--- a/cpp/demo/Ice/session/Server.cpp
+++ b/cpp/demo/Ice/session/Server.cpp
@@ -7,7 +7,7 @@
//
// **********************************************************************
-#include <Ice/Ice.h>
+#include <Ice/Ice.h> // XXX Get rid of this, see other comments.
#include <SessionFactoryI.h>
#include <ReapThread.h>
@@ -18,10 +18,12 @@ int
run(int argc, char* argv[], const Ice::CommunicatorPtr& communicator)
{
Ice::ObjectAdapterPtr adapter = communicator->createObjectAdapter("SessionFactory");
+
ReapThreadPtr reaper = ReapThread::instance();
+ reaper->start();
+
adapter->add(new SessionFactoryI, Ice::stringToIdentity("SessionFactory"));
adapter->activate();
- reaper->start();
communicator->waitForShutdown();
reaper->terminate();
diff --git a/cpp/demo/Ice/session/Session.ice b/cpp/demo/Ice/session/Session.ice
index 564960b9a23..3b887b64f26 100644
--- a/cpp/demo/Ice/session/Session.ice
+++ b/cpp/demo/Ice/session/Session.ice
@@ -15,15 +15,21 @@ module Demo
interface Hello
{
- // Send a greeting.
nonmutating void sayHello();
};
//
// The session object. This is used to create per-session objects on
-// behalf of the client. If it is not refreshed on a periodic basis it
+// behalf of the client. If it is not refreshed on a periodic basis, it
// will be automatically reclaimed by the session factory.
-///
+//
+// XXX Comment is wrong. First, sessions are not reclaimed - they are
+// destroyed. Second, the factory doesn't do this. Third, it is
+// irrelevant for the interface description to explain what
+// programming language construct exactly destroys expired
+// sessions. So this should simply read: "If it is not refreshed on a
+// periodic basis, it will be automatically destroyed."
+//
interface Session
{
//
@@ -34,18 +40,22 @@ interface Session
//
// Refresh a session. If a session is not refreshed on a regular
- // basis by the client it will be automatically destroyed.
+ // basis by the client, it will be automatically destroyed.
//
idempotent void refresh();
//
- // Destroy the session.
+ // Destroy the session explicitly.
//
void destroy();
};
interface SessionFactory
{
+ // XXX I think you should pass a user name parameter in here. The
+ // client could then first ask "What is your name"? Then the
+ // server could print messages like "Hello object #XXX says `Hello
+ // Foo!'"
Session* create();
idempotent void shutdown();
diff --git a/cpp/demo/Ice/session/SessionFactoryI.cpp b/cpp/demo/Ice/session/SessionFactoryI.cpp
index 66d27f083e8..03fa652e862 100755
--- a/cpp/demo/Ice/session/SessionFactoryI.cpp
+++ b/cpp/demo/Ice/session/SessionFactoryI.cpp
@@ -7,7 +7,7 @@
//
// **********************************************************************
-#include <Ice/Ice.h>
+#include <Ice/Ice.h> // XXX Get rid of this, SessionFactoryI.cpp must include this.
#include <SessionFactoryI.h>
#include <ReapThread.h>
@@ -25,7 +25,7 @@ SessionFactoryI::~SessionFactoryI()
SessionPrx
SessionFactoryI::create(const Ice::Current& c)
{
- Lock sync(*this);
+ Lock sync(*this); // XXX What is the mutex lock needed for?
SessionIPtr session = new SessionI;
SessionPrx proxy = SessionPrx::uncheckedCast(c.adapter->addWithUUID(session));
@@ -34,7 +34,7 @@ SessionFactoryI::create(const Ice::Current& c)
}
void
-SessionFactoryI::shutdown(const ::Ice::Current& c)
+SessionFactoryI::shutdown(const Ice::Current& c)
{
cout << "Shutting down..." << endl;
c.adapter->getCommunicator()->shutdown();
diff --git a/cpp/demo/Ice/session/SessionFactoryI.h b/cpp/demo/Ice/session/SessionFactoryI.h
index 6987d7f1d22..c46c11c1b2d 100755
--- a/cpp/demo/Ice/session/SessionFactoryI.h
+++ b/cpp/demo/Ice/session/SessionFactoryI.h
@@ -10,16 +10,18 @@
#ifndef SESSION_FACTORY_I_H
#define SESSION_FACTORY_I_H
+// XXX Missing #includes, see comment in SessionI.h.
#include <Session.h>
-class SessionFactoryI : public Demo::SessionFactory, public IceUtil::Mutex
+class SessionFactoryI : public Demo::SessionFactory, public IceUtil::Mutex // XXX What is the mutex needed for?
{
public:
+ // XXX Constructor and destructor do nothing, get rid of them.
SessionFactoryI();
virtual ~SessionFactoryI();
- virtual Demo::SessionPrx create(const ::Ice::Current&);
+ virtual Demo::SessionPrx create(const Ice::Current&);
virtual void shutdown(const Ice::Current&);
};
diff --git a/cpp/demo/Ice/session/SessionI.cpp b/cpp/demo/Ice/session/SessionI.cpp
index 4cf07aa8fa3..803c5530036 100755
--- a/cpp/demo/Ice/session/SessionI.cpp
+++ b/cpp/demo/Ice/session/SessionI.cpp
@@ -7,8 +7,8 @@
//
// **********************************************************************
-#include <Ice/Ice.h>
-#include <SessionFactoryI.h>
+#include <Ice/Ice.h> // XXX Remove, see comment in SessionFactoryI.cpp
+#include <SessionFactoryI.h> // XXX Remove, not needed anywhere.
#include <SessionI.h>
using namespace std;
@@ -76,6 +76,7 @@ SessionI::destroy(const Ice::Current& c)
_destroy = true;
+ // XXX Why "#"? Use "The session with the identity `...' is now destroyed."
cout << "The session #" << Ice::identityToString(c.id) << " is now destroyed." << endl;
try
{
diff --git a/cpp/demo/Ice/session/SessionI.h b/cpp/demo/Ice/session/SessionI.h
index 7988e46078d..30f5b923832 100755
--- a/cpp/demo/Ice/session/SessionI.h
+++ b/cpp/demo/Ice/session/SessionI.h
@@ -10,6 +10,11 @@
#ifndef SESSION_I_H
#define SESSION_I_H
+// XXX Missing #includes. The #includes for the header must be
+// self-contained, i.e. there must be no problem if you just do
+// #include<SessionFactoryI.h> from an empty .cpp file. (I know that
+// some other demos don't follow this rule, but they need to be
+// fixed. We shouldn't propagate such mistakes into new demos.)
#include <Session.h>
#include <list>
@@ -32,7 +37,8 @@ private:
SessionI();
IceUtil::Time _timestamp; // The last time the session was refreshed.
-
+
+ // XXX This needs to be a static, otherwise hello objects from different client have the same ID.
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.
bool _destroy;