From 0259ca7d673550df405e29e617a533b87e4f24ca Mon Sep 17 00:00:00 2001
From: g0dil <g0dil@wiback.org>
Date: Thu, 13 Aug 2009 12:42:42 +0000
Subject: [PATCH] Socket: Fix handle.state() on invalid handles Utils/Console:
Catch exceptions when closing client PPI: Add throttle tracing PPI: Fix Event
and IOEvent throttling
---
PPI/Connectors.cc | 31 +++++++++++++++++++++------
PPI/Connectors.cci | 9 ++++++--
PPI/Connectors.hh | 5 +++--
PPI/Events.cc | 8 +++++--
PPI/Events.cci | 3 ++-
PPI/Events.hh | 1 +
PPI/IOEvent.ct | 5 ++++-
PPI/ModuleManager.cc | 8 ++++++-
PPI/SocketSink.ct | 2 +-
Socket/ClientSocketHandle.cti | 6 +++++-
Socket/ProtocolClientSocketHandle.cti | 6 +++++-
Socket/ProtocolServerSocketHandle.cti | 6 +++++-
Socket/ServerSocketHandle.cti | 6 +++++-
Socket/SocketHandle.cti | 6 +++++-
Utils/Console/Server.cc | 10 ++++++++-
15 files changed, 89 insertions(+), 23 deletions(-)
diff --git a/PPI/Connectors.cc b/PPI/Connectors.cc
index cf241d5ec..ce44e0d23 100644
--- a/PPI/Connectors.cc
+++ b/PPI/Connectors.cc
@@ -84,7 +84,7 @@ prefix_ void senf::ppi::connector::Connector::trace(Packet const & p, char const
return;
SENF_LOG_BLOCK(({
std::string type (prettyName(p.typeId().id()));
- log << "PPI trace: " << label << " 0x" << std::hex << p.id() << " "
+ log << "PPI packet trace: " << label << " 0x" << std::hex << p.id() << " "
<< type.substr(21, type.size()-22) << " on " << & module() << " "
<< prettyName(typeid(module())) << " connector 0x" << this << "\n";
if (traceState_ == TRACE_CONTENTS)
@@ -92,6 +92,17 @@ prefix_ void senf::ppi::connector::Connector::trace(Packet const & p, char const
}));
}
+prefix_ void senf::ppi::connector::Connector::throttleTrace(char const * label,
+ char const * type)
+{
+ if (traceState_ == NO_TRACING)
+ return;
+ SENF_LOG_BLOCK(({
+ log << "PPI throttling trace: " << label << " " << type << " on " << & module()
+ << " " << prettyName(typeid(module())) << " connector 0x" << this << "\n";
+ }));
+}
+
namespace senf { namespace ppi { namespace connector {
SENF_CONSOLE_REGISTER_ENUM_MEMBER(
@@ -121,20 +132,23 @@ namespace {
"A log message is generated whenever the packet traverses a connector. The\n"
"TRACE_IDS log message has the following format:\n"
"\n"
- " PPI trace: <packet-id> <packet-type> <direction>\n"
+ " PPI packet trace: <direction> <packet-id> <packet-type>\n"
+ " on <module-id> <module-type> connector <connector-id>\n"
+ " PPI throttling trace: <direction> <throttle-msg>\n"
" on <module-id> <module-type> connector <connector-id>\n"
"\n"
"The fields are:\n"
"\n"
+ " direction 'IN' for packets/throttle notifications entering the module,\n"
+ " 'OUT' for packets/throttle notifications leaving it\n"
" packet-id Numeric unique packet id. This value is unique for packets\n"
" alive at the same time, packets at different times may (and\n"
" will) share id's\n"
" packet-type The type of the packet header\n"
- " direction 'INCOMING' for packets entering the module, 'OUTGOING' for\n"
- " packets leaving it\n"
" module-id Unique module id\n"
" module-type Type of the module the packet is sent to/from\n"
- " connector-id Unique connector id\n");
+ " connector-id Unique connector id\n"
+ " throttle-msg Type of throttling event\n");
senf::ppi::ModuleManager::instance().consoleDir()
.add("tracing", SENF_FNP(void, senf::ppi::connector::Connector::tracing,
@@ -197,7 +211,8 @@ prefix_ void senf::ppi::connector::PassiveConnector::notifyUnthrottle()
remoteThrottled_ = false;
if (!nativeThrottled_)
emitUnthrottle();
- }
+ } else
+ throttleTrace("OUT", "not forwarding unthrottle event");
}
///////////////////////////////////////////////////////////////////////////
@@ -214,6 +229,7 @@ prefix_ void senf::ppi::connector::ActiveConnector::v_init()
prefix_ void senf::ppi::connector::ActiveConnector::notifyThrottle()
{
+ throttleTrace("IN ", "throttle");
if (! throttled_) {
throttled_ = true;
if (throttleCallback_)
@@ -227,6 +243,7 @@ prefix_ void senf::ppi::connector::ActiveConnector::notifyThrottle()
prefix_ void senf::ppi::connector::ActiveConnector::notifyUnthrottle()
{
+ throttleTrace("IN ", "unthrottle");
if (throttled_) {
throttled_ = false;
if (unthrottleCallback_)
@@ -256,7 +273,7 @@ prefix_ senf::Packet senf::ppi::connector::InputConnector::operator()()
queue_.pop_back();
v_dequeueEvent();
}
- trace(p, "INCOMING");
+ trace(p, "IN ");
return p;
}
diff --git a/PPI/Connectors.cci b/PPI/Connectors.cci
index f480b02d3..ec587ceff 100644
--- a/PPI/Connectors.cci
+++ b/PPI/Connectors.cci
@@ -110,12 +110,14 @@ prefix_ bool senf::ppi::connector::PassiveConnector::throttled()
prefix_ void senf::ppi::connector::PassiveConnector::emitThrottle()
{
+ throttleTrace("OUT", "throttle");
if (connected())
peer().notifyThrottle();
}
prefix_ void senf::ppi::connector::PassiveConnector::emitUnthrottle()
{
+ throttleTrace("OUT", "unthrottle");
if (connected()) {
peer().notifyUnthrottle();
v_unthrottleEvent();
@@ -150,7 +152,8 @@ prefix_ void senf::ppi::connector::PassiveConnector::throttle()
if (!throttled()) {
nativeThrottled_ = true;
emitThrottle();
- }
+ } else
+ nativeThrottled_ = true;
}
prefix_ void senf::ppi::connector::PassiveConnector::unthrottle()
@@ -176,6 +179,8 @@ prefix_ void senf::ppi::connector::PassiveConnector::emit()
SENF_ASSERT(callback_ && "senf::ppi::connector::PassiveConnector: missing onRequest()");
if (!throttled())
callback_();
+ else
+ throttleTrace("IN ", "queueing packet");
}
///////////////////////////////////////////////////////////////////////////
@@ -286,7 +291,7 @@ prefix_ senf::ppi::connector::InputConnector & senf::ppi::connector::OutputConne
prefix_ void senf::ppi::connector::OutputConnector::operator()(Packet const & p)
{
- trace(p, "OUTGOING");
+ trace(p, "OUT");
if (connected())
peer().enqueue(p);
}
diff --git a/PPI/Connectors.hh b/PPI/Connectors.hh
index 002154ee4..e6b8c07af 100644
--- a/PPI/Connectors.hh
+++ b/PPI/Connectors.hh
@@ -183,6 +183,7 @@ namespace connector {
void connect(Connector & target);
void trace(Packet const & p, char const * label);
+ void throttleTrace(char const * label, char const * type);
private:
virtual std::type_info const & packetTypeID();
@@ -247,11 +248,11 @@ namespace connector {
private:
virtual void v_init();
- // Called by the routing to change the remote throttling state
+ // Called by the routing to change the throttling state from forwarding routes
void notifyThrottle(); ///< Forward a throttle notification to this connector
void notifyUnthrottle(); ///< Forward an unthrottle notification to this connector
- // Internal members to emit throttling notifications
+ // Internal members to emit throttling notifications to the connected peer
void emitThrottle();
void emitUnthrottle();
diff --git a/PPI/Events.cc b/PPI/Events.cc
index 3525d3db9..7aa02989e 100644
--- a/PPI/Events.cc
+++ b/PPI/Events.cc
@@ -43,13 +43,17 @@ prefix_ void senf::ppi::EventDescriptor::notifyUnthrottle()
for (; i != i_end; ++i)
if ((*i)->throttled())
break;
- if (i == i_end)
- enabled(true);
+ if (i != i_end)
+ return;
+ throttled_ = false;
+ enabled(true);
}
prefix_ void senf::ppi::EventDescriptor::enabled(bool v)
{
SENF_ASSERT(v_isRegistered() && "Module::registerEvent() call missing");
+ if (throttled_ && v)
+ return;
if (v && ! enabled_)
v_enable();
else if (! v && enabled_)
diff --git a/PPI/Events.cci b/PPI/Events.cci
index 1e50eb733..3800f201a 100644
--- a/PPI/Events.cci
+++ b/PPI/Events.cci
@@ -44,7 +44,7 @@ prefix_ bool senf::ppi::EventDescriptor::enabled()
// protected members
prefix_ senf::ppi::EventDescriptor::EventDescriptor()
- : enabled_(false)
+ : enabled_(false), throttled_(false)
{}
////////////////////////////////////////
@@ -52,6 +52,7 @@ prefix_ senf::ppi::EventDescriptor::EventDescriptor()
prefix_ void senf::ppi::EventDescriptor::notifyThrottle()
{
+ throttled_ = true;
enabled(false);
}
diff --git a/PPI/Events.hh b/PPI/Events.hh
index 3f2fc25b2..39e9d486c 100644
--- a/PPI/Events.hh
+++ b/PPI/Events.hh
@@ -87,6 +87,7 @@ namespace ppi {
void registerRoute(ForwardingRoute & route);
bool enabled_;
+ bool throttled_;
typedef std::vector<ForwardingRoute*> Routes;
Routes routes_;
diff --git a/PPI/IOEvent.ct b/PPI/IOEvent.ct
index 0f94c3c3b..e33c06fe9 100644
--- a/PPI/IOEvent.ct
+++ b/PPI/IOEvent.ct
@@ -36,7 +36,10 @@ prefix_ void senf::ppi::IOEvent::set(Handle handle, unsigned events)
if (handle) {
fd_ = senf::scheduler::get_descriptor(handle);
event_.events(events).handle(fd_);
- event_.enable();
+ if (enabled())
+ event_.enable();
+ else
+ event_.disable();
}
else {
event_.disable();
diff --git a/PPI/ModuleManager.cc b/PPI/ModuleManager.cc
index 9e890b7f1..77f190c17 100644
--- a/PPI/ModuleManager.cc
+++ b/PPI/ModuleManager.cc
@@ -80,7 +80,13 @@ prefix_ senf::ppi::ModuleManager::ModuleManager()
.add("dump", senf::membind(&ModuleManager::dumpModules, this))
.doc("Dump complete PPI structure\n"
"The dump will contain one paragraph for each module. The first line gives module\n"
- "information, additional lines list all connectors and their peers (if connected).");
+ "information, additional lines list all connectors and their peers (if connected).\n"
+ "\n"
+ "This information can be processed by 'PPI/drawmodules.py' and 'dot' (from the\n"
+ "graphviz package) to generate a graphic representation of the module structure:\n"
+ "\n"
+ " $ echo /sys/ppi/dump | nc -q1 <host> <port> \\\n"
+ " | python PPI/drawmodules.py | dot -Tpng /dev/fd/0 >modules.png\n");
}
prefix_ void senf::ppi::ModuleManager::dumpModules(std::ostream & os)
diff --git a/PPI/SocketSink.ct b/PPI/SocketSink.ct
index 69726a28d..4864625b6 100644
--- a/PPI/SocketSink.ct
+++ b/PPI/SocketSink.ct
@@ -125,7 +125,7 @@ prefix_ void senf::ppi::module::PassiveSocketSink<Writer>::write()
template <class Writer>
prefix_ void senf::ppi::module::PassiveSocketSink<Writer>::checkThrottle()
{
- if (handle_)
+ if (handle_.valid())
input.unthrottle();
else
input.throttle();
diff --git a/Socket/ClientSocketHandle.cti b/Socket/ClientSocketHandle.cti
index 079229dbf..76e6d73f0 100644
--- a/Socket/ClientSocketHandle.cti
+++ b/Socket/ClientSocketHandle.cti
@@ -377,7 +377,11 @@ template <class SPolicy>
prefix_ void senf::ClientSocketHandle<SPolicy>::state(SocketStateMap & map, unsigned lod)
{
map["handle"] = prettyName(typeid(*this));
- this->body().state(map, lod);
+ if (this->valid()) {
+ map["valid"] << "true";
+ this->body().state(map,lod);
+ } else
+ map["valid"] << "false";
}
template <class SPolicy>
diff --git a/Socket/ProtocolClientSocketHandle.cti b/Socket/ProtocolClientSocketHandle.cti
index efdb9349a..1d49c702f 100644
--- a/Socket/ProtocolClientSocketHandle.cti
+++ b/Socket/ProtocolClientSocketHandle.cti
@@ -91,7 +91,11 @@ senf::ProtocolClientSocketHandle<SocketProtocol>::state(SocketStateMap & map,
unsigned lod)
{
map["handle"] = prettyName(typeid(*this));
- this->body().state(map,lod);
+ if (this->valid()) {
+ map["valid"] << "true";
+ this->body().state(map,lod);
+ } else
+ map["valid"] << "false";
}
template <class SocketProtocol>
diff --git a/Socket/ProtocolServerSocketHandle.cti b/Socket/ProtocolServerSocketHandle.cti
index 2cd775314..26c2b6965 100644
--- a/Socket/ProtocolServerSocketHandle.cti
+++ b/Socket/ProtocolServerSocketHandle.cti
@@ -91,7 +91,11 @@ senf::ProtocolServerSocketHandle<SocketProtocol>::state(SocketStateMap & map,
unsigned lod)
{
map["handle"] = prettyName(typeid(*this));
- this->body().state(map,lod);
+ if (this->valid()) {
+ map["valid"] << "true";
+ this->body().state(map,lod);
+ } else
+ map["valid"] << "false";
}
template <class SocketProtocol>
diff --git a/Socket/ServerSocketHandle.cti b/Socket/ServerSocketHandle.cti
index 132625d78..34b850b82 100644
--- a/Socket/ServerSocketHandle.cti
+++ b/Socket/ServerSocketHandle.cti
@@ -147,7 +147,11 @@ template <class SPolicy>
prefix_ void senf::ServerSocketHandle<SPolicy>::state(SocketStateMap & map, unsigned lod)
{
map["handle"] = prettyName(typeid(*this));
- this->body().state(map,lod);
+ if (this->valid()) {
+ map["valid"] << "true";
+ this->body().state(map,lod);
+ } else
+ map["valid"] << "false";
}
template <class SPolicy>
diff --git a/Socket/SocketHandle.cti b/Socket/SocketHandle.cti
index 310dfddcc..bc3f7be93 100644
--- a/Socket/SocketHandle.cti
+++ b/Socket/SocketHandle.cti
@@ -175,7 +175,11 @@ prefix_ void senf::SocketHandle<SPolicy>::state(SocketStateMap & map, unsigned l
// the type name and therefore show the \e static policy of the
// socket handle.
map["handle"] << prettyName(typeid(*this));
- body().state(map,lod);
+ if (valid()) {
+ map["valid"] << "true";
+ body().state(map,lod);
+ } else
+ map["valid"] << "false";
}
template <class SPolicy>
diff --git a/Utils/Console/Server.cc b/Utils/Console/Server.cc
index 66564bb01..a1bad60e4 100644
--- a/Utils/Console/Server.cc
+++ b/Utils/Console/Server.cc
@@ -111,7 +111,15 @@ prefix_ void senf::console::Server::newClient(int event)
prefix_ void senf::console::Server::removeClient(Client & client)
{
- SENF_LOG(( "Disposing client " << client.handle().peer() ));
+ SENF_LOG_BLOCK(({
+ log << "Disposing client ";
+ try {
+ log << client.handle().peer();
+ }
+ catch (senf::SystemException ex) {
+ log << "(unknown)";
+ }
+ }));
// THIS DELETES THE CLIENT INSTANCE !!
clients_.erase(boost::intrusive_ptr<Client>(&client));
}
--
GitLab