I'm running out the door and don't really have time to finish this patch.
But this is along the lines of what I'm really looking for.
thanks,
BMS
Index: libxipc/xrl_router.hh
===================================================================
--- libxipc/xrl_router.hh (revision 11686)
+++ libxipc/xrl_router.hh (working copy)
@@ -34,7 +34,10 @@
#include "finder_constants.hh"
#include "finder_client_observer.hh"
+#include <boost/shared_ptr.hpp>
+using boost::shared_ptr;
+
class DispatchState;
class FinderClient;
@@ -190,6 +193,7 @@
/**
* Send callback (fast path).
+ * FIXME: use smart ptr.
*/
void send_callback(const XrlError& e,
XrlArgs* reply,
@@ -211,6 +215,7 @@
uint16_t finder_port);
private:
+ // XXX
XrlPFSender& get_sender(const string& target);
XrlPFSender* get_sender(const Xrl& xrl, FinderDBEntry *dbe);
@@ -224,10 +229,12 @@
list<XrlPFListener*> _listeners; // listeners
list<XrlRouterDispatchState*> _dsl; // dispatch
state
- list<XrlPFSender*> _senders; // active senders
+ list< shared_ptr<XrlPFSender> > _senders; // active senders
static uint32_t _icnt; // instance count
+ // XXX the following are mostly only used by the incomplete
+ // batch support. Stick to using unchecked pointers for now --bms
private:
typedef map<string, XI*> XIM;
typedef map<string, XrlPFSender*> SENDERS;
Index: libxipc/xrl_pf_stcp.cc
===================================================================
--- libxipc/xrl_pf_stcp.cc (revision 11686)
+++ libxipc/xrl_pf_stcp.cc (working copy)
@@ -1083,12 +1083,17 @@
void
XrlPFSTCPSender::batch_start()
{
+#if 0
_batching = true;
+#else
+ XLOG_UNREACHABLE();
+#endif
}
void
XrlPFSTCPSender::batch_stop()
{
+#if 0
_batching = false;
// If we aint got no requests, we may not be able to signal to the receiver
@@ -1100,4 +1105,7 @@
if (_writer->running() == false)
_writer->start();
+#else
+ XLOG_UNREACHABLE();
+#endif
}
Index: libxipc/xrl_pf_factory.hh
===================================================================
--- libxipc/xrl_pf_factory.hh (revision 11686)
+++ libxipc/xrl_pf_factory.hh (working copy)
@@ -27,19 +27,20 @@
#include "xrl_error.hh"
#include "xrl_pf.hh"
+#include <boost/shared_ptr.hpp>
+
+using boost::shared_ptr;
+
class XrlPFSenderFactory {
public:
static void startup();
static void shutdown();
- static XrlPFSender* create_sender(EventLoop& eventloop,
- const char* proto_colon_addr);
+ static shared_ptr<XrlPFSender> create_sender(
+ EventLoop& eventloop, const char* proto_colon_addr);
- static XrlPFSender* create_sender(EventLoop& e,
- const char* protocol,
- const char* address);
-
- static void destroy_sender(XrlPFSender* s);
+ static shared_ptr<XrlPFSender> create_sender(EventLoop& e,
+ const char* protocol, const char* address);
};
#endif // __LIBXIPC_XRL_PF_FACTORY_HH__
Index: libxipc/xrl.cc
===================================================================
--- libxipc/xrl.cc (revision 11686)
+++ libxipc/xrl.cc (working copy)
@@ -76,7 +76,7 @@
const XrlArgs& args)
: _protocol(protocol), _target(protocol_target), _command(command),
_args(args), _sna_atom(NULL), _packed_bytes(0), _argp(&_args),
- _to_finder(-1), _resolved(false), _resolved_sender(NULL)
+ _to_finder(-1), _resolved(false)
{
}
@@ -85,7 +85,7 @@
const XrlArgs& args)
: _protocol(_finder_protocol), _target(target), _command(command),
_args(args), _sna_atom(NULL), _packed_bytes(0), _argp(&_args),
- _to_finder(-1), _resolved(false), _resolved_sender(NULL)
+ _to_finder(-1), _resolved(false)
{
}
@@ -94,7 +94,7 @@
const string& command)
: _protocol(protocol), _target(protocol_target), _command(command),
_sna_atom(NULL), _packed_bytes(0), _argp(&_args), _to_finder(-1),
- _resolved(false), _resolved_sender(NULL)
+ _resolved(false)
{
}
@@ -102,7 +102,7 @@
const string& command)
: _protocol(_finder_protocol), _target(target), _command(command),
_sna_atom(NULL), _packed_bytes(0), _argp(&_args), _to_finder(-1),
- _resolved(false), _resolved_sender(NULL)
+ _resolved(false)
{
}
@@ -110,13 +110,13 @@
const char* command)
: _protocol(_finder_protocol), _target(target), _command(command),
_sna_atom(NULL), _packed_bytes(0), _argp(&_args), _to_finder(-1),
- _resolved(false), _resolved_sender(NULL)
+ _resolved(false)
{
}
Xrl::Xrl(const char* c_str) throw (InvalidString)
: _sna_atom(NULL), _packed_bytes(0), _argp(&_args),
- _to_finder(-1), _resolved(false), _resolved_sender(NULL)
+ _to_finder(-1), _resolved(false)
{
if (0 == c_str)
xorp_throw0(InvalidString);
@@ -136,7 +136,7 @@
Xrl::Xrl()
: _sna_atom(0), _packed_bytes(0), _argp(&_args), _to_finder(-1),
- _resolved(false), _resolved_sender(NULL)
+ _resolved(false)
{
}
@@ -335,8 +335,9 @@
_packed_bytes = 0;
_to_finder = -1;
_resolved = false;
- _resolved_sender = NULL;
+ _resolved_sender.reset();
+
delete _sna_atom;
_sna_atom = NULL;
}
Index: libxipc/xrl_pf.cc
===================================================================
--- libxipc/xrl_pf.cc (revision 11686)
+++ libxipc/xrl_pf.cc (working copy)
@@ -55,4 +55,5 @@
XrlPFSender::~XrlPFSender()
{
+ // XXX put a debug_msg() here; we are now deleted through shared_ptr.
}
Index: libxipc/xrl.hh
===================================================================
--- libxipc/xrl.hh (revision 11686)
+++ libxipc/xrl.hh (working copy)
@@ -26,11 +26,15 @@
#include <string>
+#include <boost/weak_ptr.hpp>
+
#include "libxorp/exceptions.hh"
#include "xrl_atom.hh"
#include "xrl_args.hh"
#include "xrl_tokens.hh"
+using boost::weak_ptr;
+
class XrlPFSender;
/**
@@ -169,9 +173,15 @@
bool resolved() const { return _resolved; }
void set_resolved(bool r) const { _resolved = r; }
- XrlPFSender *resolved_sender() const { return _resolved_sender; }
- void set_resolved_sender(XrlPFSender *s) const { _resolved_sender = s; }
+ // XXX Don't declare these two const until shared_ptr is worked out --bms
+ weak_ptr<XrlPFSender> resolved_sender() {
+ return _resolved_sender;
+ }
+ void set_resolved_sender(weak_ptr<XrlPFSender> s) {
+ _resolved_sender = s;
+ }
+
void set_target(const char* target);
private:
@@ -194,7 +204,7 @@
mutable XrlArgs* _argp; // XXX shouldn't be mutable
mutable int _to_finder;
mutable bool _resolved; // XXX ditto
- mutable XrlPFSender* _resolved_sender; // XXX ditto
+ mutable weak_ptr<XrlPFSender> _resolved_sender; // XXX ditto
static const string _finder_protocol;
};
Index: libxipc/xrl_pf.hh
===================================================================
--- libxipc/xrl_pf.hh (revision 11686)
+++ libxipc/xrl_pf.hh (working copy)
@@ -105,7 +105,15 @@
virtual bool sends_pending() const = 0;
virtual const char* protocol() const = 0;
+
+ /**
+ * Determine if the underlying transport is still open.
+ *
+ * @return true if the transport is alive.
+ */
virtual bool alive() const = 0;
+
+ // XXX Unfinished support for XRL batching.
virtual void batch_start() {}
virtual void batch_stop() {}
Index: libxipc/xrl_router.cc
===================================================================
--- libxipc/xrl_router.cc (revision 11686)
+++ libxipc/xrl_router.cc (working copy)
@@ -38,6 +38,10 @@
#include "sockutil.hh"
+// XXX should be included nested above.
+//#include <boost/shared_ptr.hpp>
+//#include <boost/weak_ptr.hpp>
+
//
// Enable this macro to enable Xrl callback checker that checks each Xrl
// callback is dispatched just once.
@@ -273,10 +277,8 @@
_fc->detach_observer(this);
_fac->set_enabled(false);
- while (_senders.empty() == false) {
- XrlPFSenderFactory::destroy_sender(_senders.front());
+ while (_senders.empty() == false)
_senders.pop_front();
- }
while (_dsl.empty() == false) {
delete _dsl.front();
@@ -389,7 +391,8 @@
bool direct_call)
{
try {
- XrlPFSender* s = get_sender(xrl, const_cast<FinderDBEntry*>(dbe));
+ shared_ptr<XrlPFSender> s =
+ get_sender(xrl, const_cast<FinderDBEntry*>(dbe));
if (s == 0) {
// Notify Finder client that result was bad.
_fc->uncache_result(dbe);
@@ -400,10 +403,11 @@
const Xrl& x = dbe->xrls().front();
x.set_args(xrl);
- if (s) {
+ if (s != 0) {
trace_xrl("Sending ", x);
return s->send(x, direct_call,
- callback(this, &XrlRouter::send_callback, s, cb));
+ callback(this, &XrlRouter::send_callback,
+ s.get(), cb));
}
cb->dispatch(XrlError(SEND_FAILED, "sender not instantiated"), 0);
} catch (const InvalidString&) {
@@ -412,24 +416,37 @@
return false;
}
-XrlPFSender*
+// Return 0 or locked shared_ptr.
+
+shared_ptr<XrlPFSender>
XrlRouter::get_sender(const Xrl& xrl, FinderDBEntry* dbe)
{
const Xrl& x = dbe->xrls().front();
- XrlPFSender* s = NULL;
+ shared_ptr<XrlPFSender> s;
- // Use the cache pointer to the sender.
+ // Try to use the cached pointer to the sender.
if (xrl.resolved()) {
- s = xrl.resolved_sender();
+ weak_ptr<XrlPFSender> w = xrl.resolved_sender();
- if (s->alive())
+ // Check if cached weak_ptr to sender is still valid; we may
+ // have lost a race on the XRLDB.
+ // Check if transport layer is still up for this sender.
+ // If good to go, return shared pointer for which we
+ // still hold the lock.
+ s = w.lock();
+ if (s != 0 && s->alive())
return s;
+ // We lost the race.
+
+ // XXX Sanity check that this sender's protocol and endpoint
+ // addresses are the same as they were when the Xrl was
+ // instantiated.
XLOG_ASSERT(s->protocol() == x.protocol());
XLOG_ASSERT(s->address() == x.target());
xrl.set_resolved(false);
- xrl.set_resolved_sender(NULL);
+ xrl.set_resolved_sender(0);
}
// Find a new sender.
@@ -448,13 +465,13 @@
XLOG_INFO("Sender died (protocol = \"%s\", address = \"%s\")",
s->protocol(), s->address().c_str());
- XrlPFSenderFactory::destroy_sender(s);
+
_senders.erase(i);
_senders2.erase(xrl.target());
break;
}
- s = NULL;
+ s.reset();
// create sender
while (dbe->xrls().size()) {
@@ -462,7 +479,7 @@
s = XrlPFSenderFactory::create_sender(_e, x.protocol().c_str(),
x.target().c_str());
- if (s)
+ if (s != 0)
break;
XLOG_ERROR("Could not create XrlPFSender for protocol = \"%s\" "
@@ -471,15 +488,18 @@
dbe->pop_front();
}
- if (!s)
- return NULL;
+ // Unable to instantiate.
+ if (s == 0)
+ return 0;
+
+ // New sender instantiated, take lock and record state.
const Xrl& front = dbe->xrls().front();
XLOG_ASSERT(s->protocol() == front.protocol());
XLOG_ASSERT(s->address() == front.target());
_senders.push_back(s);
- _senders2[xrl.target()] = s;
+ _senders2[xrl.target()] = s.get();
// Don't do this here as it is set in the Xrl that is stored with
// the finder client, so is not used. But if the connection needs
@@ -506,7 +526,7 @@
if (e == XrlError::OKAY()) {
const Xrl& xrl = ds->xrl();
xrl.set_resolved(false);
- xrl.set_resolved_sender(NULL);
+ xrl.set_resolved_sender(0);
if (send_resolved(xrl, dbe, ds->cb(), false) == false) {
// We tried to force sender to send xrl and it declined the
// opportunity. This should only happen when it's out of buffer
@@ -710,25 +730,44 @@
debug_msg("Finder target ready event: \"%s\"\n", tgt_name.c_str());
}
+//
+// XXX This is unfinished code related to a batch XRL implementation.
+// It is unfortunately incompatible, straight off, with using refcounts to
+// manage XrlPFSender's lifecycle, so it needs to be revisited later by
+// an interested party.
+// Also, the overloading of the get_sender() method name is potentially
+// confusing. --bms
+
void
XrlRouter::batch_start(const string& target)
{
+#if 0
XrlPFSender& sender = get_sender(target);
sender.batch_start();
+#else
+ XLOG_UNREACHABLE();
+ UNUSED(target);
+#endif
}
void
XrlRouter::batch_stop(const string& target)
{
+#if 0
XrlPFSender& sender = get_sender(target);
sender.batch_stop();
+#else
+ XLOG_UNREACHABLE();
+ UNUSED(target);
+#endif
}
XrlPFSender&
XrlRouter::get_sender(const string& target)
{
+#if 0
XrlPFSender* s = 0;
SENDERS::iterator i = _senders2.find(target);
@@ -737,6 +776,12 @@
s = i->second;
return *s;
+#else
+ XLOG_UNREACHABLE();
+ XrlPFSender* s = 0;
+ return *s;
+ UNUSED(target);
+#endif
}
Index: libxipc/xrl_pf_factory.cc
===================================================================
--- libxipc/xrl_pf_factory.cc (revision 11686)
+++ libxipc/xrl_pf_factory.cc (working copy)
@@ -38,7 +38,7 @@
// real cost, unlike InProc and SUDP, so we maintain a cache of
// STCP senders with one per sender destination address.
-XrlPFSender*
+shared_ptr<XrlPFSender>
XrlPFSenderFactory::create_sender(EventLoop& eventloop,
const char* protocol,
const char* address)
@@ -58,7 +58,7 @@
return 0;
}
-XrlPFSender*
+shared_ptr<XrlPFSender>
XrlPFSenderFactory::create_sender(EventLoop& eventloop,
const char* protocol_colon_address)
{
@@ -74,15 +74,6 @@
}
void
-XrlPFSenderFactory::destroy_sender(XrlPFSender* s)
-{
- debug_msg("destroying sender pf = \"%s\", addr = \"%s\"\n",
- s->protocol(), s->address().c_str());
-
- delete s;
-}
-
-void
XrlPFSenderFactory::startup()
{
}
_______________________________________________
Xorp-hackers mailing list
[email protected]
http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-hackers
