[
https://issues.apache.org/jira/browse/QPID-2337?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12845672#action_12845672
]
Tom McCubbin commented on QPID-2337:
------------------------------------
After spending a few hours on this (yeah...should have been a bit quicker...) I
came to the same conclusion. I was happy to find the
SubscriptionManager::start() method that starts a new thread from whence msgs
can be dispatched. Until I found out that start() which is handled via the
qpid::client::Dispatcher just fires a thread which calls
qpid::client::Dispatcher::run(). Run in turn may throw a ClosedException,
TransportFailure, or std::exception.
When in a separate thread, any exceptions are unhandled and result in the
untimely demise of that thread and every other thread, unless I make wild
presumptions in my unhandled exception handler, which is not realistic.
Furthermore, ClosedException and TransportFailure do not call the
failoverHandler, yet std::exceptions do? This seems quite awkward. If you start
w/out a broker available, it reliably calls the failoverHandler(). If however
you crash the server, it throws and you are done.
As any old code-monkey does, I assume at some point the connected server will
crash. Not if, but when. This behaviour is not acceptable.
To fix it, I did two very simple things. Both change
qpidc-0.5/src/qpid/client/Dispatcher.cpp (no header change)
1) I changed the qpid::client::Dispatcher::run() method so if there is an
exception, ANY TYPE, and if there is a failoverHandler registered that handler
will be called, and no exception will be thrown
Added below: (+++ and the 'e' for the first 2 catch clauses )
catch (const ClosedException& e) {
+++ if ( failoverHandler ) {
+++ QPID_LOG(debug, QPID_MSG(session.getId() << " failover: " << e.what()));
+++ failoverHandler();
+++ } else {
QPID_LOG(debug, QPID_MSG(session.getId() << ": closed by peer"));
+++ }
}
catch (const TransportFailure& e) {
+++ if ( failoverHandler ) {
+++ QPID_LOG(debug, QPID_MSG(session.getId() << " failover: " << e.what()));
+++ failoverHandler();
+++ } else {
QPID_LOG(info, QPID_MSG(session.getId() << ": transport failure"));
throw;
+++ }
}
catch (const std::exception& e) {
if ( failoverHandler ) {
QPID_LOG(debug, QPID_MSG(session.getId() << " failover: " <<
e.what()));
failoverHandler();
} else {
QPID_LOG(error, session.getId() << " error: " << e.what());
throw;
}
}
2) when the qpid::client::Dispatcher::start() method is called, if there is no
failoverHandler registered, I register an anonymous dummy function I added to
the same .cpp file. Looks like this.
+++namespace {
+++ void dumbHandler() { }
+++}
void Dispatcher::start()
{
+++ if ( ! failoverHandler )
+++ registerFailoverHandler( dumbHandler );
worker = Thread(this);
}
This is working wonderfully for me so far. Yeah, its not perfect, but hey its
quicker than adding my own thread which can have the try/catch code to wrap the
run() method.
In practice, if you register a failureCallback on the qpid::client::Connection
to handle these errors you need not worry about any other handler for
SubscriptionManager(s) and the code above will bring you quickly to Shangri La.
If you want, you can still register a failureCallback on the
SubscriptionManager and provided you use my modified run() method, you will be
ok as well.
If anyone wants a patch, let me know.
> SubscriptionManager::start() does not handle exceptions from dispatch
> ---------------------------------------------------------------------
>
> Key: QPID-2337
> URL: https://issues.apache.org/jira/browse/QPID-2337
> Project: Qpid
> Issue Type: Bug
> Components: C++ Client
> Affects Versions: 0.5, 0.6
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Fix For: 0.7
>
>
> ...so any exceptions thrown (e.g. when connection is lost) will cause client
> process to terminate if start() is used.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
