[asterisk-scf-commits] asterisk-scf/integration/bridging.git branch "retry_deux" updated.
Commits to the Asterisk SCF project code repositories
asterisk-scf-commits at lists.digium.com
Mon Apr 9 13:18:55 CDT 2012
branch "retry_deux" has been updated
via 439e365875ee9565130a14898529fe895b84785e (commit)
from 8cd4348ac2d38f19a7d33b2af7b784ab99603cac (commit)
Summary of changes:
.../BridgeService/BridgeReplicatorIf.ice | 57 ++-
src/BridgeImpl.cpp | 513 ++++++++++----------
src/BridgeManagerImpl.cpp | 366 +++++++++------
src/BridgeManagerImpl.h | 3 +-
src/BridgeReplicatorStateListenerI.cpp | 12 +-
src/BridgeServiceConfig.h | 3 +
src/MediaSplicer.cpp | 14 +-
src/SessionListener.cpp | 6 +-
src/SessionWrapper.cpp | 19 +-
test/TestBridging.cpp | 3 +-
10 files changed, 549 insertions(+), 447 deletions(-)
- Log -----------------------------------------------------------------
commit 439e365875ee9565130a14898529fe895b84785e
Author: Brent Eagles <beagles at digium.com>
Date: Mon Apr 9 15:46:23 2012 -0230
Fix up several exception try/catch blocks. Added assertions for catching
... and ObjectNotExistExceptions. Also ONEs are now swallowed in several
places instead of being allowed to propagate. I think there needs to be
another pass to examine all servant methods, not just the ones that involve
recent OperationContext changes.
There are also some changes related to review feedback.
diff --git a/slice/AsteriskSCF/Replication/BridgeService/BridgeReplicatorIf.ice b/slice/AsteriskSCF/Replication/BridgeService/BridgeReplicatorIf.ice
index 7f08258..8bc215a 100644
--- a/slice/AsteriskSCF/Replication/BridgeService/BridgeReplicatorIf.ice
+++ b/slice/AsteriskSCF/Replication/BridgeService/BridgeReplicatorIf.ice
@@ -76,6 +76,39 @@ enum MediaOperationReplicationPolicy
Reconstruct /* Media operations are reconstructed on the replica */
};
+
+/**
+ * Items to support replication of contexts. The strategy here is to simply
+ * replicate a context along with a coded value to indicate what operation
+ * the context was for. The coded value is a simple integral constant for
+ * the time being. This is a little bit of a heavy-handed and brute force,
+ * but after trying other approaches, I prefer this one.
+ */
+
+/**
+ * Context replication constants. The values indicate the method that
+ * received the incoming context.
+ */
+enum ContextCodes
+{
+ CreateBridge,
+ AddSession,
+ ReplaceSession,
+ RemoveSession,
+ Shutdown
+};
+
+/**
+ * State item for replicating incoming contexts for key
+ * operations. Typically these are only replicted when an interface method
+ * has a broad range of side-effects that are difficult to recover from.
+ */
+class ContextStateItem extends ReplicatedStateItem
+{
+ ContextCodes code;
+ AsteriskSCF::System::V1::OperationContext operationContext;
+};
+
/**
*
* A bridge establishes connections between media objects associated with sessions.
@@ -171,10 +204,13 @@ sequence<AsteriskSCF::SessionCommunications::V1::BridgeManagerListener*> BridgeM
*/
class PartyIdHooks
{
- AsteriskSCF::SessionCommunications::ExtensionPoints::V1::ReceivedConnectedLinePartyIdHookSeq receivedConnectedLineHooks;
- AsteriskSCF::SessionCommunications::ExtensionPoints::V1::ForwardingConnectedLinePartyIdHookSeq forwardingConnectedLineHooks;
+ AsteriskSCF::SessionCommunications::ExtensionPoints::V1::ReceivedConnectedLinePartyIdHookSeq
+ receivedConnectedLineHooks;
+ AsteriskSCF::SessionCommunications::ExtensionPoints::V1::ForwardingConnectedLinePartyIdHookSeq
+ forwardingConnectedLineHooks;
AsteriskSCF::SessionCommunications::ExtensionPoints::V1::ForwardingCallerPartyIdHookSeq forwardingCallerHooks;
- AsteriskSCF::SessionCommunications::ExtensionPoints::V1::ForwardingRedirectionsPartyIdHookSeq forwardingRedirectionsHooks;
+ AsteriskSCF::SessionCommunications::ExtensionPoints::V1::ForwardingRedirectionsPartyIdHookSeq
+ forwardingRedirectionsHooks;
};
/**
@@ -228,6 +264,7 @@ class BridgeListenerStateItem extends ReplicatedStateItem
AsteriskSCF::SessionCommunications::V1::BridgeListener* listener;
};
+
/**
*
* The BridgeReplicatorListener interface must be implemented by components
@@ -237,8 +274,8 @@ class BridgeListenerStateItem extends ReplicatedStateItem
**/
interface ReplicatorListener
{
- void stateRemoved(AsteriskSCF::System::V1::OperationContext operationContext, Ice::StringSeq itemKeys);
- void stateSet(AsteriskSCF::System::V1::OperationContext operationContext, ReplicatedStateItemSeq items);
+ idempotent void stateRemoved(AsteriskSCF::System::V1::OperationContext operationContext, Ice::StringSeq itemKeys);
+ idempotent void stateSet(AsteriskSCF::System::V1::OperationContext operationContext, ReplicatedStateItemSeq items);
};
/**
@@ -249,11 +286,13 @@ interface ReplicatorListener
**/
interface Replicator
{
- void addListener(AsteriskSCF::System::V1::OperationContext operationContext, ReplicatorListener* listener);
- void removeListener(AsteriskSCF::System::V1::OperationContext operationContext, ReplicatorListener* listener);
+ idempotent void addListener(AsteriskSCF::System::V1::OperationContext operationContext,
+ ReplicatorListener* listener);
+ idempotent void removeListener(AsteriskSCF::System::V1::OperationContext operationContext,
+ ReplicatorListener* listener);
- void setState(AsteriskSCF::System::V1::OperationContext operationContext, ReplicatedStateItemSeq items);
- void removeState(AsteriskSCF::System::V1::OperationContext operationContext, Ice::StringSeq items);
+ idempotent void setState(AsteriskSCF::System::V1::OperationContext operationContext, ReplicatedStateItemSeq items);
+ idempotent void removeState(AsteriskSCF::System::V1::OperationContext operationContext, Ice::StringSeq items);
idempotent ReplicatedStateItemSeq getState(Ice::StringSeq itemKeys);
idempotent ReplicatedStateItemSeq getAllState();
diff --git a/src/BridgeImpl.cpp b/src/BridgeImpl.cpp
index 3f02425..647f23a 100755
--- a/src/BridgeImpl.cpp
+++ b/src/BridgeImpl.cpp
@@ -720,6 +720,10 @@ protected:
if (!wrapper)
{
+ //
+ // TODO: Returning true early allows subsequent tasks to
+ // run. Might be a dubious practice!
+ //
mLogger(Debug) << "Unable to find matching session for, returning early with true.";
return true;
}
@@ -756,6 +760,11 @@ protected:
currentConnectedLine = updatedConnectedLine;
}
}
+ //
+ // catching and ignoring an Ice exception here might be
+ // true to the hook calls, but other exceptions really
+ // should be considered bugs.
+ //
catch (const std::exception& e)
{
mLogger(Debug) << FUNLOG << " : " << e.what();
@@ -967,16 +976,20 @@ public:
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->getProxy()->ice_exception();
}
}
}
catch (const std::exception& ex)
{
+ mLogger(Error) << ex.what() << " exception occurred when trying to get operation context data.";
cb->ice_exception(ex);
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
+ mLogger(Error) << "Unknown exception occurred when trying to get operation context data.";
cb->ice_exception();
}
}
@@ -1001,10 +1014,13 @@ public:
}
catch (const std::exception& ex)
{
+ mLogger(Error) << ex.what() << " exception occurred when trying to get operation context data.";
cb->ice_exception(ex);
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
+ mLogger(Error) << "Unknown exception occurred when trying to get operation context data.";
cb->ice_exception();
}
}
@@ -1020,18 +1036,9 @@ public:
mBridge->updateConnectedLine(mSelf, context, connectedLine);
data->getMonitor()->setCompleted();
}
- catch (const Ice::Exception& ex)
- {
- data->setException(ExceptionWrapper::create(ex));
- throw;
- }
- catch (const std::exception& ex)
- {
- data->setException(ExceptionWrapper::create(ex));
- throw;
- }
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -1058,10 +1065,9 @@ public:
mBridge->updateRedirections(mSelf, context, redirections);
data->getMonitor()->setCompleted();
}
- catch (const Ice::Exception& ex)
+ catch (const Ice::ObjectNotExistException&)
{
- data->setException(ExceptionWrapper::create(ex));
- throw;
+ assert("We should not be ignoring or passing along ONEs" == 0);
}
catch (const std::exception& ex)
{
@@ -1070,6 +1076,7 @@ public:
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -1642,7 +1649,10 @@ BridgeImpl::~BridgeImpl()
void BridgeImpl::updateConnectedLine(const SessionWrapperPtr& sourceSession,
const AsteriskSCF::System::V1::OperationContextPtr& context, const ConnectedLinePtr& connectedLine)
{
- try
+ //
+ // TODO: Is it correct to simply ignore any exceptions that occur here?
+ //
+ try
{
QueuedTasks tasks;
@@ -1676,7 +1686,8 @@ void BridgeImpl::updateRedirections(const SessionWrapperPtr& sourceSession,
// Forwards the Redirections information to the other sessions in the bridge.
// Applies forwarding hooks to the Redirecrting info before forwarding.
- tasks.push_back(new ForwardRedirectionsUpdatedTask(context, this, sourceSession->getSession(), redirections, mLogger));
+ tasks.push_back(new ForwardRedirectionsUpdatedTask(context, this, sourceSession->getSession(),
+ redirections, mLogger));
ExecutorPtr runner(new Executor(tasks, mLogger));
runner->start();
}
@@ -1699,96 +1710,86 @@ void BridgeImpl::addSessions_async(const AMD_Bridge_addSessionsPtr& callback,
const SessionWithSessionInfoSeq& sessionInfos,
const Ice::Current&)
{
- try
- {
- AddSessionsContextDataPtr data(
- getContext<AddSessionsContextDataPtr, AMD_Bridge_addSessionsPtr>(
- mOperationContextCache, operationContext, callback));
+ AddSessionsContextDataPtr data(
+ getContext<AddSessionsContextDataPtr, AMD_Bridge_addSessionsPtr>(
+ mOperationContextCache, operationContext, callback));
- if (data)
+ if (data)
+ {
+ try
{
- try
+ mSessions->reap();
+ SessionSeq sessions;
+ sessions.resize(sessionInfos.size());
+ std::transform(sessionInfos.begin(), sessionInfos.end(), sessions.begin(), extractSession);
+ if (sessions.empty())
{
- mSessions->reap();
- SessionSeq sessions;
- sessions.resize(sessionInfos.size());
- std::transform(sessionInfos.begin(), sessionInfos.end(), sessions.begin(), extractSession);
- if (sessions.empty())
+ if (callback)
{
- if (callback)
- {
- data->getProxy()->ice_response();
- }
- return;
+ data->getProxy()->ice_response();
}
- checkSessions(sessions);
- statePreCheck();
- mLogger(Trace) << FUNLOG << ": adding " << sessions.size() << " sessions";
- SessionsTrackerPtr tracker(new SessionsTracker);
+ return;
+ }
+ checkSessions(sessions);
+ statePreCheck();
+ mLogger(Trace) << FUNLOG << ": adding " << sessions.size() << " sessions";
+ SessionsTrackerPtr tracker(new SessionsTracker);
- QueuedTasks tasks;
- tasks.push_back(new UnplugMedia(operationContext, this));
- tasks.push_back(new SetBridgeTask(operationContext, mSessions, mPrx, mSessionListenerPrx, sessions, tracker));
- tasks.push_back(new AddToListeners(operationContext, mListeners, tracker, getCookies()));
- tasks.push_back(new SetAndGetSessionControllerTask(operationContext,
- mObjAdapter, mSessions, sessions, this, mLogger));
+ QueuedTasks tasks;
+ tasks.push_back(new UnplugMedia(operationContext, this));
+ tasks.push_back(new SetBridgeTask(operationContext, mSessions, mPrx, mSessionListenerPrx, sessions, tracker));
+ tasks.push_back(new AddToListeners(operationContext, mListeners, tracker, getCookies()));
+ tasks.push_back(new SetAndGetSessionControllerTask(operationContext,
+ mObjAdapter, mSessions, sessions, this, mLogger));
- //
- // The new sessions being added to the bridge need to have their connected line set and
- // forwarded to the existing sessions.
- //
- // TODO: This very much like the SessionOperations which are
- // applied with the "visit" operation.
- //
- for (SessionWithSessionInfoSeq::const_iterator infoIter = sessionInfos.begin();
- infoIter != sessionInfos.end(); ++infoIter)
- {
- if (infoIter->info && infoIter->info->sessionOwner)
- {
- tasks.push_back(new UpdateConnectedLineTask(operationContext, this, infoIter->sessionProxy,
- new ConnectedLine(infoIter->info->sessionOwner->ids), mLogger));
- tasks.push_back(new ForwardConnectedLineTask(operationContext, this, infoIter->sessionProxy, mLogger));
- }
- }
-
- //The existing sessions need to have their stored connected line forwarded to the new
- //sessions
- SessionSeq existingSessions = getSessions()->getSessionSeq();
- for (SessionSeq::const_iterator sessionIter = existingSessions.begin();
- sessionIter != existingSessions.end(); ++sessionIter)
+ //
+ // The new sessions being added to the bridge need to have their connected line set and
+ // forwarded to the existing sessions.
+ //
+ // TODO: This very much like the SessionOperations which are
+ // applied with the "visit" operation.
+ //
+ for (SessionWithSessionInfoSeq::const_iterator infoIter = sessionInfos.begin();
+ infoIter != sessionInfos.end(); ++infoIter)
+ {
+ if (infoIter->info && infoIter->info->sessionOwner)
{
- tasks.push_back(new ForwardConnectedLineTask(operationContext, this, *sessionIter, mLogger));
+ tasks.push_back(new UpdateConnectedLineTask(operationContext, this, infoIter->sessionProxy,
+ new ConnectedLine(infoIter->info->sessionOwner->ids), mLogger));
+ tasks.push_back(new ForwardConnectedLineTask(operationContext, this, infoIter->sessionProxy, mLogger));
}
-
- tasks.push_back(new GenericAMDCallback<AMD_Bridge_addSessionsPtr>(data->getProxy(), tracker));
- tasks.push_back(new SetupMedia(operationContext, this));
- tasks.push_back(new ConnectTelephonyEventsTask(operationContext, this, sessions, mLogger));
- tasks.push_back(new UpdateTask(operationContext, this));
- ExecutorPtr executor(new Executor(tasks, mLogger));
- executor->start();
- //
- // When the operations have all completed, that last task will take care of handling
- // the callback. It's all left withing the try/catch in the event something happens during
- // task startup.
- //
- }
- catch (const std::exception& ex)
- {
- data->getProxy()->ice_exception(ex);
}
- catch (...)
+
+ //The existing sessions need to have their stored connected line forwarded to the new
+ //sessions
+ SessionSeq existingSessions = getSessions()->getSessionSeq();
+ for (SessionSeq::const_iterator sessionIter = existingSessions.begin();
+ sessionIter != existingSessions.end(); ++sessionIter)
{
- data->getProxy()->ice_exception();
+ tasks.push_back(new ForwardConnectedLineTask(operationContext, this, *sessionIter, mLogger));
}
+
+ tasks.push_back(new GenericAMDCallback<AMD_Bridge_addSessionsPtr>(data->getProxy(), tracker));
+ tasks.push_back(new SetupMedia(operationContext, this));
+ tasks.push_back(new ConnectTelephonyEventsTask(operationContext, this, sessions, mLogger));
+ tasks.push_back(new UpdateTask(operationContext, this));
+ ExecutorPtr executor(new Executor(tasks, mLogger));
+ executor->start();
+ //
+ // When the operations have all completed, that last task will take care of handling
+ // the callback. It's all left withing the try/catch in the event something happens during
+ // task startup.
+ //
+ }
+ catch (const std::exception& ex)
+ {
+ data->getProxy()->ice_exception(ex);
+ }
+ catch (...)
+ {
+ assert("If we got here, really bad things have happened!" == 0);
+ data->getProxy()->ice_exception();
}
- }
- catch (const std::exception& ex)
- {
- callback->ice_exception(ex);
- }
- catch (...)
- {
- callback->ice_exception();
}
}
@@ -1799,73 +1800,63 @@ void BridgeImpl::removeSessions_async(const AMD_Bridge_removeSessionsPtr& callba
const AsteriskSCF::System::V1::OperationContextPtr& operationContext, const SessionSeq& sessions,
const Ice::Current&)
{
- try
+ RemoveSessionsContextDataPtr data(
+ getContext<RemoveSessionsContextDataPtr, AMD_Bridge_removeSessionsPtr>(
+ mOperationContextCache, operationContext, callback));
+ if (data)
{
- RemoveSessionsContextDataPtr data(
- getContext<RemoveSessionsContextDataPtr, AMD_Bridge_removeSessionsPtr>(
- mOperationContextCache, operationContext, callback));
- if (data)
+ try
{
- try
+ if (sessions.empty())
{
- if (sessions.empty())
- {
- data->getProxy()->ice_response();
- return;
- }
- checkSessions(sessions);
- statePreCheck();
- mSessions->reap();
-
- //
- // The shutdown of individual sessions are implemented as series of AMI requests. Once initiated,
- // we allow them to proceed asynchronously and do not concern ourselves with the result.
- // The logic of shutdown should remove them either because the operations succeeded or
- // *couldn't* be accomplished because of some terminal condition. At any rate, waiting around for them
- // is pointless.
- //
- SessionsTrackerPtr removed(new SessionsTracker);
- for (SessionSeq::const_iterator i = sessions.begin(); i != sessions.end(); ++i)
- {
- SessionWrapperPtr session = mSessions->getSession(*i);
- if (session)
- {
- removed->add(session->getSession());
- mSessions->removeSession(operationContext, session->getBridgedSession());
- }
- }
- QueuedTasks tasks;
+ data->getProxy()->ice_response();
+ return;
+ }
+ checkSessions(sessions);
+ statePreCheck();
+ mSessions->reap();
- BridgeCookies cookies;
+ //
+ // The shutdown of individual sessions are implemented as series of AMI requests. Once initiated,
+ // we allow them to proceed asynchronously and do not concern ourselves with the result.
+ // The logic of shutdown should remove them either because the operations succeeded or
+ // *couldn't* be accomplished because of some terminal condition. At any rate, waiting around for them
+ // is pointless.
+ //
+ SessionsTrackerPtr removed(new SessionsTracker);
+ for (SessionSeq::const_iterator i = sessions.begin(); i != sessions.end(); ++i)
+ {
+ SessionWrapperPtr session = mSessions->getSession(*i);
+ if (session)
{
- boost::shared_lock<boost::shared_mutex> lock(mLock);
- cookies = getCookies();
+ removed->add(session->getSession());
+ mSessions->removeSession(operationContext, session->getBridgedSession());
}
- tasks.push_back(new RemoveSessionsNotify(operationContext, mListeners, removed, cookies));
- tasks.push_back(new RemoveSessionControllerTask(operationContext, mObjAdapter, removed, mLogger));
- tasks.push_back(new GenericAMDCallback<AMD_Bridge_removeSessionsPtr>(data->getProxy(), removed));
- tasks.push_back(new DisconnectTelephonyEventsTask(operationContext, this, sessions, mLogger));
- tasks.push_back(new CheckShutdown(this, mPrx));
- ExecutorPtr runner(new Executor(tasks, mLogger));
- runner->start();
- }
- catch (const std::exception& ex)
- {
- data->getProxy()->ice_exception(ex);
}
- catch (...)
+ QueuedTasks tasks;
+
+ BridgeCookies cookies;
{
- data->getProxy()->ice_exception();
+ boost::shared_lock<boost::shared_mutex> lock(mLock);
+ cookies = getCookies();
}
+ tasks.push_back(new RemoveSessionsNotify(operationContext, mListeners, removed, cookies));
+ tasks.push_back(new RemoveSessionControllerTask(operationContext, mObjAdapter, removed, mLogger));
+ tasks.push_back(new GenericAMDCallback<AMD_Bridge_removeSessionsPtr>(data->getProxy(), removed));
+ tasks.push_back(new DisconnectTelephonyEventsTask(operationContext, this, sessions, mLogger));
+ tasks.push_back(new CheckShutdown(this, mPrx));
+ ExecutorPtr runner(new Executor(tasks, mLogger));
+ runner->start();
+ }
+ catch (const std::exception& ex)
+ {
+ data->getProxy()->ice_exception(ex);
+ }
+ catch (...)
+ {
+ assert("If we got here, really bad things have happened!" == 0);
+ data->getProxy()->ice_exception();
}
- }
- catch (const std::exception& ex)
- {
- callback->ice_exception(ex);
- }
- catch (...)
- {
- callback->ice_exception();
}
}
@@ -1958,10 +1949,9 @@ void BridgeImpl::shutdown(const AsteriskSCF::System::V1::OperationContextPtr& op
mSessions = 0;
data->getMonitor()->setCompleted();
}
- catch (const Ice::Exception& ex)
+ catch (const Ice::ObjectNotExistException& ex)
{
- data->setException(ExceptionWrapper::create(ex));
- throw;
+ assert("We should not be ignoring or rethrowing ONEs" == 0); // If we get here, fix it!
}
catch (const std::exception& ex)
{
@@ -1970,6 +1960,7 @@ void BridgeImpl::shutdown(const AsteriskSCF::System::V1::OperationContextPtr& op
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -2059,8 +2050,17 @@ void BridgeImpl::addListener(const AsteriskSCF::System::V1::OperationContextPtr&
}
data->getMonitor()->setCompleted();
}
- catch (const Ice::Exception& ex)
+ catch (const Ice::UserException& ex)
+ {
+ mLogger(Error) << FILELOG << "Uncaught user exception, probably indicating an implementation bug.";
+ assert(false); // If we get here, fix it!
+ data->setException(ExceptionWrapper::create(ex));
+ throw;
+ }
+ catch (const Ice::ObjectNotExistException& ex)
{
+ mLogger(Error) << FILELOG << "Unhandled ONE, probably indicating an implementation bug.";
+ assert(false); // If we get here, fix it!
data->setException(ExceptionWrapper::create(ex));
throw;
}
@@ -2071,6 +2071,7 @@ void BridgeImpl::addListener(const AsteriskSCF::System::V1::OperationContextPtr&
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -2114,10 +2115,9 @@ void BridgeImpl::removeListener(const AsteriskSCF::System::V1::OperationContextP
}
data->getMonitor()->setCompleted();
}
- catch (const Ice::Exception& ex)
+ catch (const Ice::ObjectNotExistException&)
{
- data->setException(ExceptionWrapper::create(ex));
- throw;
+ assert("We should not be rethrowing or ignoring ONEs" == 0); // If we get here, fix it!
}
catch (const std::exception& ex)
{
@@ -2126,6 +2126,7 @@ void BridgeImpl::removeListener(const AsteriskSCF::System::V1::OperationContextP
}
catch (...)
{
+ assert("A non Ice exception here is a bug" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -2141,120 +2142,110 @@ void BridgeImpl::replaceSession_async(const AMD_Bridge_replaceSessionPtr& callba
const SessionWithSessionInfoSeq& newSessionInfos,
const Ice::Current& current)
{
- try
+ ReplaceSessionContextDataPtr data(getContext<ReplaceSessionContextDataPtr,
+ AMD_Bridge_replaceSessionPtr>(
+ mOperationContextCache, context,
+ callback));
+ if (data)
{
- ReplaceSessionContextDataPtr data(getContext<ReplaceSessionContextDataPtr,
- AMD_Bridge_replaceSessionPtr>(
- mOperationContextCache, context,
- callback));
- if (data)
+ try
{
- try
- {
- mLogger(Trace) << FUNLOG << ":" << objectIdFromCurrent(current);
- mSessions->reap();
- SessionSeq newSessions;
- newSessions.resize(newSessionInfos.size());
- std::transform(newSessionInfos.begin(), newSessionInfos.end(), newSessions.begin(), extractSession);
- checkSessions(newSessions);
- statePreCheck();
-
- SessionWrapperPtr session = mSessions->getSession(sessionToReplace);
- //
- // If the session did not exist on this bridge, then this operation should not proceed.
- //
- if (!session)
- {
- try
- {
- throw SessionNotFound(sessionToReplace);
- }
- catch (const std::exception& ex)
- {
- data->getProxy()->ice_exception(ex);
- return;
- }
- }
- //
- // Shutdown is inheritently asynchronous (see SessionWrapper::shutdown())
- //
- SessionsTrackerPtr removeTracker(new SessionsTracker);
- removeTracker->add(session->getSession());
- session->shutdown(context, mSessionListenerPrx, new ResponseCode);
-
- //
- // Since the shutdown of the session is inherently asynchronous, the
- // rest of this is going to happen anyways, so lets remove it from the
- // list of known sessions now so it doesn't get in the way.
- //
- mSessions->removeSession(context, session->getBridgedSession());
+ mLogger(Trace) << FUNLOG << ":" << objectIdFromCurrent(current);
+ mSessions->reap();
+ SessionSeq newSessions;
+ newSessions.resize(newSessionInfos.size());
+ std::transform(newSessionInfos.begin(), newSessionInfos.end(), newSessions.begin(), extractSession);
+ checkSessions(newSessions);
+ statePreCheck();
- SessionsTrackerPtr tracker(new SessionsTracker);
- QueuedTasks tasks;
- BridgeCookies cookies;
+ SessionWrapperPtr session = mSessions->getSession(sessionToReplace);
+ //
+ // If the session did not exist on this bridge, then this operation should not proceed.
+ //
+ if (!session)
+ {
+ try
{
- boost::shared_lock<boost::shared_mutex> lock(mLock);
- cookies = getCookies();
+ throw SessionNotFound(sessionToReplace);
}
-
- SessionSeq removedSessions;
- removedSessions.push_back(sessionToReplace);
-
- tasks.push_back(new RemoveSessionsNotify(context, mListeners, removeTracker, cookies));
- tasks.push_back(new RemoveSessionControllerTask(context, mObjAdapter, removeTracker, mLogger));
- tasks.push_back(new UnplugMedia(context, this));
- tasks.push_back(new SetBridgeTask(context, mSessions, mPrx, mSessionListenerPrx, newSessions, tracker));
- tasks.push_back(new AddToListeners(context, mListeners, tracker, cookies));
- tasks.push_back(new SetAndGetSessionControllerTask(context, mObjAdapter, mSessions, newSessions, this,
- mLogger));
-
- //The new sessions being added to the bridge need to have their connected line set and
- //forwarded to the existing sessions.
- for (SessionWithSessionInfoSeq::const_iterator infoIter = newSessionInfos.begin();
- infoIter != newSessionInfos.end(); ++infoIter)
+ catch (const std::exception& ex)
{
- if (infoIter->info && infoIter->info->sessionOwner)
- {
- tasks.push_back(new UpdateConnectedLineTask(context, this, infoIter->sessionProxy,
- new ConnectedLine(infoIter->info->sessionOwner->ids), mLogger));
- tasks.push_back(new ForwardConnectedLineTask(context, this, infoIter->sessionProxy, mLogger));
- }
+ data->getProxy()->ice_exception(ex);
+ return;
}
+ }
+ //
+ // Shutdown is inheritently asynchronous (see SessionWrapper::shutdown())
+ //
+ SessionsTrackerPtr removeTracker(new SessionsTracker);
+ removeTracker->add(session->getSession());
+ session->shutdown(context, mSessionListenerPrx, new ResponseCode);
- //The existing sessions need to have their stored connected line forwarded to the new
- //sessions
- SessionSeq existingSessions = getSessions()->getSessionSeq();
- for (SessionSeq::const_iterator sessionIter = existingSessions.begin();
- sessionIter != existingSessions.end(); ++sessionIter)
- {
- tasks.push_back(new ForwardConnectedLineTask(context, this, *sessionIter, mLogger));
- }
+ //
+ // Since the shutdown of the session is inherently asynchronous, the
+ // rest of this is going to happen anyways, so lets remove it from the
+ // list of known sessions now so it doesn't get in the way.
+ //
+ mSessions->removeSession(context, session->getBridgedSession());
- tasks.push_back(new GenericAMDCallback<AMD_Bridge_replaceSessionPtr>(data->getProxy(), tracker));
- tasks.push_back(new SetupMedia(context, this));
- tasks.push_back(new ConnectTelephonyEventsTask(context, this, newSessions, mLogger));
- tasks.push_back(new DisconnectTelephonyEventsTask(context, this, removedSessions, mLogger));
- tasks.push_back(new UpdateTask(context, this));
- ExecutorPtr executor(new Executor(tasks, mLogger));
- executor->start();
+ SessionsTrackerPtr tracker(new SessionsTracker);
+ QueuedTasks tasks;
+ BridgeCookies cookies;
+ {
+ boost::shared_lock<boost::shared_mutex> lock(mLock);
+ cookies = getCookies();
}
- catch (const std::exception& ex)
+
+ SessionSeq removedSessions;
+ removedSessions.push_back(sessionToReplace);
+
+ tasks.push_back(new RemoveSessionsNotify(context, mListeners, removeTracker, cookies));
+ tasks.push_back(new RemoveSessionControllerTask(context, mObjAdapter, removeTracker, mLogger));
+ tasks.push_back(new UnplugMedia(context, this));
+ tasks.push_back(new SetBridgeTask(context, mSessions, mPrx, mSessionListenerPrx, newSessions, tracker));
+ tasks.push_back(new AddToListeners(context, mListeners, tracker, cookies));
+ tasks.push_back(new SetAndGetSessionControllerTask(context, mObjAdapter, mSessions, newSessions, this,
+ mLogger));
+
+ //The new sessions being added to the bridge need to have their connected line set and
+ //forwarded to the existing sessions.
+ for (SessionWithSessionInfoSeq::const_iterator infoIter = newSessionInfos.begin();
+ infoIter != newSessionInfos.end(); ++infoIter)
{
- data->getProxy()->ice_exception(ex);
+ if (infoIter->info && infoIter->info->sessionOwner)
+ {
+ tasks.push_back(new UpdateConnectedLineTask(context, this, infoIter->sessionProxy,
+ new ConnectedLine(infoIter->info->sessionOwner->ids), mLogger));
+ tasks.push_back(new ForwardConnectedLineTask(context, this, infoIter->sessionProxy, mLogger));
+ }
}
- catch (...)
+
+ //The existing sessions need to have their stored connected line forwarded to the new
+ //sessions
+ SessionSeq existingSessions = getSessions()->getSessionSeq();
+ for (SessionSeq::const_iterator sessionIter = existingSessions.begin();
+ sessionIter != existingSessions.end(); ++sessionIter)
{
- data->getProxy()->ice_exception();
+ tasks.push_back(new ForwardConnectedLineTask(context, this, *sessionIter, mLogger));
}
+
+ tasks.push_back(new GenericAMDCallback<AMD_Bridge_replaceSessionPtr>(data->getProxy(), tracker));
+ tasks.push_back(new SetupMedia(context, this));
+ tasks.push_back(new ConnectTelephonyEventsTask(context, this, newSessions, mLogger));
+ tasks.push_back(new DisconnectTelephonyEventsTask(context, this, removedSessions, mLogger));
+ tasks.push_back(new UpdateTask(context, this));
+ ExecutorPtr executor(new Executor(tasks, mLogger));
+ executor->start();
+ }
+ catch (const std::exception& ex)
+ {
+ data->getProxy()->ice_exception(ex);
+ }
+ catch (...)
+ {
+ assert("If we got here, really bad things have happened!" == 0);
+ data->getProxy()->ice_exception();
}
- }
- catch (const std::exception& ex)
- {
- callback->ice_exception(ex);
- }
- catch (...)
- {
- callback->ice_exception();
}
}
@@ -2278,10 +2269,9 @@ void BridgeImpl::setCookies(const AsteriskSCF::System::V1::OperationContextPtr&
}
pushUpdate(update);
}
- catch (const Ice::Exception& ex)
+ catch (const Ice::ObjectNotExistException&)
{
- data->setException(ExceptionWrapper::create(ex));
- throw;
+ assert("We should not be ignoring or rethrowing ONEs" == 0);
}
catch (const std::exception& ex)
{
@@ -2290,6 +2280,7 @@ void BridgeImpl::setCookies(const AsteriskSCF::System::V1::OperationContextPtr&
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -2327,10 +2318,9 @@ void BridgeImpl::removeCookies(const AsteriskSCF::System::V1::OperationContextPt
pushUpdate(update);
data->getMonitor()->setCompleted();
}
- catch (const Ice::Exception& ex)
+ catch (const Ice::ObjectNotExistException&)
{
- data->setException(ExceptionWrapper::create(ex));
- throw;
+ assert("We should not be ignoring or rethrowing ONEs" == 0);
}
catch (const std::exception& ex)
{
@@ -2339,6 +2329,7 @@ void BridgeImpl::removeCookies(const AsteriskSCF::System::V1::OperationContextPt
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
diff --git a/src/BridgeManagerImpl.cpp b/src/BridgeManagerImpl.cpp
index 0c7d3de..98cac33 100755
--- a/src/BridgeManagerImpl.cpp
+++ b/src/BridgeManagerImpl.cpp
@@ -81,7 +81,8 @@ public:
const AsteriskSCF::System::V1::OperationContextPtr&,
const SessionPrx& source,
const SessionSeq& endpoints,
- const BridgeListenerPrx& listener, const CallerPtr& callerID, const RedirectionsPtr& redirects, const Ice::Current&);
+ const BridgeListenerPrx& listener, const CallerPtr& callerID, const RedirectionsPtr& redirects,
+ const Ice::Current&);
void addListener(const AsteriskSCF::System::V1::OperationContextPtr& operationContext,
const BridgeManagerListenerPrx& listener, const Ice::Current& current);
@@ -148,7 +149,10 @@ private:
void reap();
void statePreCheck(const string&);
- void update();
+ //
+ // update() should be implemented as nothrow.
+ //
+ void update(const AsteriskSCF::System::V1::OperationContextPtr& context);
};
typedef IceUtil::Handle<BridgeManagerImpl> BridgeManagerImplPtr;
@@ -252,123 +256,146 @@ void BridgeManagerImpl::createBridge_async(const AMD_BridgeManager_createBridgeP
if (data)
{
+ //
+ // At this point, we should be the first and only thread for the
+ // specific context.
+ //
+ AsteriskSCF::Replication::BridgeService::V1::ContextStateItemPtr contextState(
+ new AsteriskSCF::Replication::BridgeService::V1::ContextStateItem);
+ contextState->code = AsteriskSCF::Replication::BridgeService::V1::CreateBridge;
+ contextState->operationContext = context;
+
try
{
mLogger(Trace) << FUNLOG << ":" << objectIdFromCurrent(current);
boost::unique_lock<boost::shared_mutex> lock(mLock);
statePreCheck(BOOST_CURRENT_FUNCTION);
reap();
-
-
- string stringId = string("bridge.") + IceUtil::generateUUID();
- Ice::Identity id(mAdapter->getCommunicator()->stringToIdentity(stringId));
- BridgePrx prx(BridgePrx::uncheckedCast(mAdapter->createProxy(id)));
- AsteriskSCF::SessionCommunications::V1::BridgeListenerSeq listeners(mState->defaultBridgeListeners);
- if (listener)
+ try
{
- listeners.push_back(listener);
- }
-
- //
- // "sessions" is a const, so we need to make a local non-const copy that can be modified
- // by the hooks if need be.
- //
- SessionSeq initialSessions(sessions);
+ string stringId = string("bridge.") + IceUtil::generateUUID();
+ Ice::Identity id(mAdapter->getCommunicator()->stringToIdentity(stringId));
+ BridgePrx prx(BridgePrx::uncheckedCast(mAdapter->createProxy(id)));
+ AsteriskSCF::SessionCommunications::V1::BridgeListenerSeq listeners(mState->defaultBridgeListeners);
+ if (listener)
+ {
+ listeners.push_back(listener);
+ }
- //
- // We create a local outside of the block that initializes and runs the hooks if they are
- // present. If this is '0' later on in the code, it signifies the bridge creation extension
- // point mechanism was not initialized.
- //
- BridgeCreationHookDataPtr hookData;
- BridgeInfo info;
- info.proxy = prx;
- if (mCreationExtension)
- {
//
- // Set up hook data with initial values and run the hooks!
+ // "sessions" is a const, so we need to make a local non-const copy that can be modified
+ // by the hooks if need be.
//
- hookData = new BridgeCreationHookData;
- hookData->bridge = prx;
- hookData->listeners = listeners;
- hookData->initialSessions = sessions;
- hookData = mCreationExtension->runHooks(hookData);
+ SessionSeq initialSessions(sessions);
//
- // Update locals with the result of running the hooks.
+ // We create a local outside of the block that initializes and runs the hooks if they are
+ // present. If this is '0' later on in the code, it signifies the bridge creation extension
+ // point mechanism was not initialized.
//
- listeners = hookData->listeners;
- initialSessions = hookData->initialSessions;
- info.decoratingPrx = hookData->bridge;
- if (mLogger.isEnabledFor(Trace))
+ BridgeCreationHookDataPtr hookData;
+ BridgeInfo info;
+ info.proxy = prx;
+ if (mCreationExtension)
{
- dump(mLogger, hookData);
+ //
+ // Set up hook data with initial values and run the hooks!
+ //
+ hookData = new BridgeCreationHookData;
+ hookData->bridge = prx;
+ hookData->listeners = listeners;
+ hookData->initialSessions = sessions;
+ hookData = mCreationExtension->runHooks(hookData);
+
+ //
+ // Update locals with the result of running the hooks.
+ //
+ listeners = hookData->listeners;
+ initialSessions = hookData->initialSessions;
+ info.decoratingPrx = hookData->bridge;
+ if (mLogger.isEnabledFor(Trace))
+ {
+ dump(mLogger, hookData);
+ }
}
- }
- //
- // The bridge listener manager is a wrapper/helper class that manages the listeners and
- // propogates the bridge events. We separate it's instantiation from the construction
- // of the bridge itself as this is a natural area of refinement and extension.
- //
- BridgeListenerMgrPtr mgr(new BridgeListenerMgr(mAdapter->getCommunicator(), stringId, info.decoratingPrx));
+ //
+ // The bridge listener manager is a wrapper/helper class that manages the listeners and
+ // propogates the bridge events. We separate it's instantiation from the construction
+ // of the bridge itself as this is a natural area of refinement and extension.
+ //
+ BridgeListenerMgrPtr mgr(new BridgeListenerMgr(mAdapter->getCommunicator(), stringId, info.decoratingPrx));
- //
- // Now we can get down to the creation of the bridge servant itself. Note that the
- // initialization is still not really complete as we need to set up and cookies,
- // initial sessions, etc that may have been defined or added as part of the createBridge
- // call or the bridge creation hooks.
- //
- BridgeServantPtr bridge = BridgeServant::create(stringId, mAdapter, listeners, mgr,
- mPartyIdExtensionPoint->getHooks(), mReplicationContext->getReplicator(), mLogger);
+ //
+ // Now we can get down to the creation of the bridge servant itself. Note that the
+ // initialization is still not really complete as we need to set up and cookies,
+ // initial sessions, etc that may have been defined or added as part of the createBridge
+ // call or the bridge creation hooks.
+ //
+ BridgeServantPtr bridge = BridgeServant::create(stringId, mAdapter, listeners, mgr,
+ mPartyIdExtensionPoint->getHooks(), mReplicationContext->getReplicator(), mLogger);
- Ice::ObjectPrx obj = mAdapter->add(bridge, id);
+ Ice::ObjectPrx obj = mAdapter->add(bridge, id);
- mLogger(Info) << objectIdFromCurrent(current) << ": creating new bridge " << obj->ice_toString() << "." ;
+ mLogger(Info) << objectIdFromCurrent(current) << ": creating new bridge " << obj->ice_toString() << "." ;
- //
- // Finish updating BridgeInfo struct.
- //
- info.servant = bridge;
+ //
+ // Finish updating BridgeInfo struct.
+ //
+ info.servant = bridge;
- //
- // It's very important to note that the bridge servant will not have it's own proxy!
- // NOTE: This method is probably misnamed and misleading. The bridge servant was added to
- // object adapter above. It might be a better idea to move adding the servant to the adapter
- // into the activate method.
- //
- bridge->activate(info.decoratingPrx, stringId);
- mBridges.push_back(info);
-
- if (hookData)
- {
- bridge->setCookiesImpl(hookData->cookies);
- }
+ //
+ // It's very important to note that the bridge servant will not have it's own proxy!
+ // NOTE: This method is probably misnamed and misleading. The bridge servant was added to
+ // object adapter above. It might be a better idea to move adding the servant to the adapter
+ // into the activate method.
+ //
+ bridge->activate(info.decoratingPrx, stringId);
+ mBridges.push_back(info);
+
+ if (hookData)
+ {
+ bridge->setCookiesImpl(hookData->cookies);
+ }
- //
- // There are some finalization tasks that may be performed asynchronously (replication etc.)
- //
- QueuedTasks tasks;
+ //
+ // There are some finalization tasks that may be performed asynchronously (replication etc.)
+ //
+ QueuedTasks tasks;
- //
- // If there are some sessions that need to be added to the bridge immediately
- // upon creation, create some tasks to add to the queue.
- //
- if (!initialSessions.empty())
+ //
+ // If there are some sessions that need to be added to the bridge immediately
+ // upon creation, create some tasks to add to the queue.
+ //
+ if (!initialSessions.empty())
+ {
+ bridge->getAddSessionsTasks(tasks, context, source, initialSessions, callerID, redirects);
+ }
+
+ tasks.push_back(new FinishUp(data->getProxy(), mListeners, info.decoratingPrx));
+ ExecutorPtr runner(new Executor(tasks, mLogger));
+ runner->start();
+ }
+ catch (const Ice::ObjectNotExistException&)
{
- bridge->getAddSessionsTasks(tasks, context, source, initialSessions, callerID, redirects);
+ //
+ // XXX Besides asserting or rethrowing, we should be
+ // handling these in the appropriate places. Since these
+ // code changes are occurring for a different reason, I'm
+ // placing these catch/asserts as placemarkers for what
+ // should be a follow up JIRA task.
+ //
+ assert("We should not be ignoring these" == 0);
}
-
- tasks.push_back(new FinishUp(data->getProxy(), mListeners, info.decoratingPrx));
- ExecutorPtr runner(new Executor(tasks, mLogger));
- runner->start();
}
catch (const std::exception& ex)
{
+ mLogger(Error) << "FUNLOG" << "An unhandled " << ex.what() << " is being rethrown, please contact support.";
data->getProxy()->ice_exception(ex);
}
catch (...)
- {
+ {
+ assert("If we got here, really bad things have happened!" == 0);
data->getProxy()->ice_exception();
}
}
@@ -385,25 +412,34 @@ void BridgeManagerImpl::addListener(const AsteriskSCF::System::V1::OperationCont
mLogger(Trace) << FUNLOG << ":" << objectIdFromCurrent(current);
boost::unique_lock<boost::shared_mutex> lock(mLock);
statePreCheck(BOOST_CURRENT_FUNCTION);
- if (!mListeners)
+ try
{
- mSourceProxy = BridgeManagerPrx::uncheckedCast( mAdapter->createProxy(current.id));
- mListeners = new BridgeManagerListenerMgr(mAdapter->getCommunicator(), mName, mSourceProxy);
+ if (!mListeners)
+ {
+ mSourceProxy = BridgeManagerPrx::uncheckedCast( mAdapter->createProxy(current.id));
+ mListeners = new BridgeManagerListenerMgr(mAdapter->getCommunicator(), mName, mSourceProxy);
+ }
+ mListeners->addListener(listener);
+ //
+ // TODO: I think this may hold a lock while doing an RPC. Not a
+ // great idea. While we might use asynchronous (or unreliable)
+ // invocations later on, it could change at any time and break
+ // this code.
+ //
+ update(context);
+ data->getMonitor()->setCompleted();
+ }
+ catch (const Ice::ObjectNotExistException&)
+ {
+ //
+ // XXX Besides asserting or rethrowing, we should be
+ // handling these in the appropriate places. Since these
+ // code changes are occurring for a different reason, I'm
+ // placing these catch/asserts as placemarkers for what
+ // should be a follow up JIRA task.
+ //
+ assert("We should not be ignoring ONE's" == 0);
}
- mListeners->addListener(listener);
- //
- // TODO: I think this may hold a lock while doing an RPC. Not a
- // great idea. While we might use asynchronous (or unreliable)
- // invocations later on, it could change at any time and break
- // this code.
- //
- update();
- data->getMonitor()->setCompleted();
- }
- catch (const Ice::Exception& ex)
- {
- data->setException(ExceptionWrapper::create(ex));
- throw;
}
catch (const std::exception& ex)
{
@@ -412,6 +448,7 @@ void BridgeManagerImpl::addListener(const AsteriskSCF::System::V1::OperationCont
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -429,26 +466,35 @@ void BridgeManagerImpl::removeListener(const AsteriskSCF::System::V1::OperationC
mLogger(Trace) << FUNLOG << ":" << objectIdFromCurrent(current);
boost::unique_lock<boost::shared_mutex> lock(mLock);
statePreCheck(BOOST_CURRENT_FUNCTION);
- if (!mListeners)
+ try
+ {
+ if (!mListeners)
+ {
+ mSourceProxy = BridgeManagerPrx::uncheckedCast(mAdapter->createProxy(current.id));
+ mListeners = new BridgeManagerListenerMgr(mAdapter->getCommunicator(), mName, mSourceProxy);
+ }
+ mListeners->removeListener(listener);
+ //
+ // TODO: I think this may hold a lock while doing an RPC. Not a
+ // great idea. While we might use asynchronous (or unreliable)
+ // invocations later on, it could change at any time and break
+ // this code.
+ // -- repeated from above.
+ //
+ update(context);
+ data->getMonitor()->setCompleted();
+ }
+ catch (const Ice::ObjectNotExistException&)
{
- mSourceProxy = BridgeManagerPrx::uncheckedCast(mAdapter->createProxy(current.id));
- mListeners = new BridgeManagerListenerMgr(mAdapter->getCommunicator(), mName, mSourceProxy);
+ //
+ // XXX Besides asserting or rethrowing, we should be
+ // handling these in the appropriate places. Since these
+ // code changes are occurring for a different reason, I'm
+ // placing these catch/asserts as placemarkers for what
+ // should be a follow up JIRA task.
+ //
+ assert("We should not be ignoring ONE's" == 0);
}
- mListeners->removeListener(listener);
- //
- // TODO: I think this may hold a lock while doing an RPC. Not a
- // great idea. While we might use asynchronous (or unreliable)
- // invocations later on, it could change at any time and break
- // this code.
- // -- repeated from above.
- //
- update();
- data->getMonitor()->setCompleted();
- }
- catch (const Ice::Exception& ex)
- {
- data->setException(ExceptionWrapper::create(ex));
- throw;
}
catch (const std::exception& ex)
{
@@ -457,6 +503,7 @@ void BridgeManagerImpl::removeListener(const AsteriskSCF::System::V1::OperationC
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -487,22 +534,23 @@ void BridgeManagerImpl::addDefaultBridgeListener(const AsteriskSCF::System::V1::
mLogger(Trace) << FUNLOG << ": adding new listener " << objectIdFromCurrent(current);
mState->defaultBridgeListeners.push_back(newListener);
}
- update();
+ update(context);
data->getMonitor()->setCompleted();
- }
- catch (const Ice::Exception& ex)
- {
- data->setException(ExceptionWrapper::create(ex));
- throw;
+ //
+ // Inappropriate ONEs should not really be an issue here, we are simply modifying a local
+ // data member. Exceptions from replications shouldn't get here.
+ //
}
catch (const std::exception& ex)
{
+ mLogger(Error) << FUNLOG << "An unhandled " << ex.what() << " is being rethrown, please contact support.";
data->setException(ExceptionWrapper::create(ex));
throw;
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
@@ -530,15 +578,9 @@ void BridgeManagerImpl::removeDefaultBridgeListener(
mState->defaultBridgeListeners.erase(remove_if(mState->defaultBridgeListeners.begin(),
mState->defaultBridgeListeners.end(),
IdentityComparePredicate<BridgeListenerPrx>(toRemove)), mState->defaultBridgeListeners.end());
- update();
+ update(context);
data->getMonitor()->setCompleted();
}
-
- catch (const Ice::Exception& ex)
- {
- data->setException(ExceptionWrapper::create(ex));
- throw;
- }
catch (const std::exception& ex)
{
data->setException(ExceptionWrapper::create(ex));
@@ -546,13 +588,19 @@ void BridgeManagerImpl::removeDefaultBridgeListener(
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
+ throw;
}
}
}
BridgeSeq BridgeManagerImpl::listBridges(const Ice::Current& current)
{
+ //
+ // Inherently idempotent, so we don't need to worry about operation
+ // contexts here.
+ //
mLogger(Trace) << FUNLOG << ":" << objectIdFromCurrent(current);
boost::unique_lock<boost::shared_mutex> lock(mLock);
reap();
@@ -571,11 +619,20 @@ void BridgeManagerImpl::shutdown(const AsteriskSCF::System::V1::OperationContext
ContextDataPtr data(checkAndThrow(mOperationCache, operationContext));
if (data)
{
+ //
+ // In some respects, shutdown is inherently idempotent.
try
{
-
mLogger(Trace) << FUNLOG << ":" << objectIdFromCurrent(current);
boost::unique_lock<boost::shared_mutex> lock(mLock);
+
+ //
+ // This pseudo-state-pre-check is not like state checks in other objects. We do not
+ // throw ObjectNotExist exceptions. In fact, this should be avoided at all costs.
+ // The current standard deployment is one where this bridge manager is a "persistent"
+ // and "well known" object. None of the servant methods that for this object type
+ // should throw ONEs
+ //
if (mState->runningState == AsteriskSCF::Replication::BridgeService::V1::ShuttingDown)
{
mLogger(Trace) << FUNLOG << ": already shutting down.";
@@ -592,6 +649,8 @@ void BridgeManagerImpl::shutdown(const AsteriskSCF::System::V1::OperationContext
}
mLogger(Info) << current.adapter->getCommunicator()->identityToString(current.id) << ": shutting down." ;
mState->runningState = AsteriskSCF::Replication::BridgeService::V1::ShuttingDown;
+
+
reap();
for (vector<BridgeInfo>::iterator i = mBridges.begin(); i != mBridges.end(); ++i)
{
@@ -606,7 +665,7 @@ void BridgeManagerImpl::shutdown(const AsteriskSCF::System::V1::OperationContext
{
if (mCreationExtensionPointServicePrx)
{
- mCreationExtensionPointServicePrx->unregister(AsteriskSCF::Operations::createContext());
+ mCreationExtensionPointServicePrx->unregister(AsteriskSCF::Operations::createContext(operationContext));
}
}
catch (const std::exception& ex)
@@ -617,10 +676,9 @@ void BridgeManagerImpl::shutdown(const AsteriskSCF::System::V1::OperationContext
mState->runningState = AsteriskSCF::Replication::BridgeService::V1::Destroyed;
data->getMonitor()->setCompleted();
}
- catch (const Ice::Exception& ex)
+ catch (const Ice::ObjectNotExistException&)
{
- data->setException(ExceptionWrapper::create(ex));
- throw;
+ assert("We should not be ignoring ONE's" == 0);
}
catch (const std::exception& ex)
{
@@ -629,6 +687,7 @@ void BridgeManagerImpl::shutdown(const AsteriskSCF::System::V1::OperationContext
}
catch (...)
{
+ assert("If we got here, really bad things have happened!" == 0);
data->setException(ExceptionWrapper::create("Unknown unexpected exception"));
throw;
}
@@ -781,7 +840,12 @@ void BridgeManagerImpl::statePreCheck(const string& caller)
}
}
-void BridgeManagerImpl::update()
+//
+// update() should not allow exceptions to escape it. It would interfere
+// with the operation of functions that call it. Exceptions thrown therein
+// should alter the state of replication only and then be swallowed up.
+//
+void BridgeManagerImpl::update(const AsteriskSCF::System::V1::OperationContextPtr& context)
{
if (mReplicationContext->isReplicating())
{
@@ -790,11 +854,23 @@ void BridgeManagerImpl::update()
seq.push_back(getState());
try
{
- mReplicationContext->getReplicator()->setState(AsteriskSCF::Operations::createContext(), seq);
+ mReplicationContext->getReplicator()->setState(AsteriskSCF::Operations::createContext(context), seq);
}
- catch (const Ice::Exception& ex)
+ //
+ // TODO: Does an ONE here imply something with respect to FT setup
+ // and should it be handled differently.
+ //
+ catch (const std::exception& ex)
+ {
+ //
+ // We will swallow this exception for the time being.
+ //
+ mLogger(Error) << "Caught exception while trying to replicate bridge manager state: " << ex.what();
+ }
+ catch (...)
{
- mLogger(Warning) << "Caught exception while trying to replicate bridge manager state: " << ex.what();
+ mLogger(Error) << "Critical error: Unknown and unexpected exception occurred when replicating!";
+ assert(false);
}
}
}
diff --git a/src/BridgeManagerImpl.h b/src/BridgeManagerImpl.h
index ab2b6aa..b07fa12 100644
--- a/src/BridgeManagerImpl.h
+++ b/src/BridgeManagerImpl.h
@@ -54,7 +54,8 @@ public:
virtual std::string getID() = 0;
- virtual void createBridgeReplica(const AsteriskSCF::Replication::BridgeService::V1::BridgeStateItemPtr& bridgeState) = 0;
+ virtual void createBridgeReplica(
+ const AsteriskSCF::Replication::BridgeService::V1::BridgeStateItemPtr& bridgeState) = 0;
virtual void removeBridge(const std::string& bridgeId) = 0;
};
diff --git a/src/BridgeReplicatorStateListenerI.cpp b/src/BridgeReplicatorStateListenerI.cpp
index d3a46be..c9300e6 100644
--- a/src/BridgeReplicatorStateListenerI.cpp
+++ b/src/BridgeReplicatorStateListenerI.cpp
@@ -51,7 +51,8 @@ public:
map<string, ReplicatedStateItemPtr>::iterator entry = mItems.find(bridgeKey);
if (entry == mItems.end())
{
- mLogger(Trace) << "no items left, and the bridge itself is no longer in item dictionary, it is safe to remove the bridge";
+ mLogger(Trace) << "no items left, and the bridge itself is no longer in item dictionary, it "
+ "is safe to remove the bridge";
mManager->removeBridge(bridgeKey);
}
}
@@ -321,8 +322,10 @@ public:
}
if (!found)
{
- mLogger(Warning) << "received a " << bridgeListener->ice_id() << " (id: " << bridgeListener->key << ")"
- << " update for a session on a bridge that does not exist: " << bridgeListener->bridgeId;
+ mLogger(Warning) << "received a " << bridgeListener->ice_id() << " (id: "
+ << bridgeListener->key << ")"
+ << " update for a session on a bridge that does not exist: "
+ << bridgeListener->bridgeId;
}
continue;
@@ -342,7 +345,8 @@ private:
}
-ReplicatorListenerPtr AsteriskSCF::BridgeService::createStateListener(const Logger& logger, const BridgeManagerServantPtr& manager)
+ReplicatorListenerPtr AsteriskSCF::BridgeService::createStateListener(const Logger& logger,
+ const BridgeManagerServantPtr& manager)
{
return new ListenerI(logger, manager);
}
diff --git a/src/BridgeServiceConfig.h b/src/BridgeServiceConfig.h
index 63f7db7..3e6b240 100644
--- a/src/BridgeServiceConfig.h
+++ b/src/BridgeServiceConfig.h
@@ -76,6 +76,9 @@ private:
#define FUNLOG \
'(' << BOOST_CURRENT_FUNCTION << ')'
+#define FILELOG \
+ __FILE__ << ':' << __LINE__ << ' '
+
inline std::string objectIdFromCurrent(const Ice::Current& current)
{
if (current.adapter)
diff --git a/src/MediaSplicer.cpp b/src/MediaSplicer.cpp
index eeba5fd..12e4a95 100755
--- a/src/MediaSplicer.cpp
+++ b/src/MediaSplicer.cpp
@@ -392,15 +392,15 @@ private:
std::map<std::string, std::string> mConnections;
};
-QueuedTasks createMediaConnectTasks(const AsteriskSCF::System::V1::OperationContextPtr& context, const SessionWrapperPtr& session,
- const AsteriskSCF::SessionCommunications::V1::SessionPrx& sessionPrx,
- const MediaConnectorIPtr& peer, const MediaSplicerIPtr& splicer, bool mixer);
+QueuedTasks createMediaConnectTasks(const AsteriskSCF::System::V1::OperationContextPtr& context,
+ const SessionWrapperPtr& session, const AsteriskSCF::SessionCommunications::V1::SessionPrx& sessionPrx,
+ const MediaConnectorIPtr& peer, const MediaSplicerIPtr& splicer, bool mixer);
class MediaSplicerI : public boost::enable_shared_from_this<MediaSplicerI>
{
public:
- MediaSplicerI(const Ice::CommunicatorPtr& communicator, const string& bridgeId, const ReplicatorSmartPrx& replicator, const Logger& logger,
- const Ice::ObjectAdapterPtr& adapter) :
+ MediaSplicerI(const Ice::CommunicatorPtr& communicator, const string& bridgeId,
+ const ReplicatorSmartPrx& replicator, const Logger& logger, const Ice::ObjectAdapterPtr& adapter) :
mCommunicator(communicator),
mBridgeId(bridgeId),
mReplicator(replicator),
@@ -411,8 +411,8 @@ public:
}
//
- // This operation is called internally when the operations for initializing a MediaConnectorBuilder instance have
- // completed.
+ // This operation is called internally when the operations for
+ // initializing a MediaConnectorBuilder instance have completed.
//
MediaConnectorPtr createConnector(const SessionWrapperPtr& session, const MediaConnectorBuilderPtr& data)
{
diff --git a/src/SessionListener.cpp b/src/SessionListener.cpp
index 60f09b4..0e6dd24 100644
--- a/src/SessionListener.cpp
+++ b/src/SessionListener.cpp
@@ -281,10 +281,9 @@ public:
}
data->getMonitor()->setCompleted();
}
- catch (const Ice::Exception& ex)
+ catch (const Ice::ObjectNotExistException&)
{
- data->setException(ExceptionWrapper::create(ex));
- throw;
+ assert("We should not be ignoring or rethrowing ONEs" == 0);
}
catch (const std::exception& ex)
{
@@ -293,6 +292,7 @@ public:
}
catch (...)
{
+ assert(false);
data->setException(ExceptionWrapper::create("Unexpected unknown exception"));
throw;
}
diff --git a/src/SessionWrapper.cpp b/src/SessionWrapper.cpp
index 2419727..b3d8b5b 100755
--- a/src/SessionWrapper.cpp
+++ b/src/SessionWrapper.cpp
@@ -65,23 +65,9 @@ public:
{
ex.ice_throw();
}
- catch (const Ice::RequestFailedException& x)
+ catch (const Ice::ObjectNotExistException& x)
{
- //
- // These exceptions indicate something is wrong with the
- // request on this object. We should not expect that it
- // could ever succeed. ObjectNotExistException is included
- // in this catch.
- //
- mLogger(Warning) << "exception when calling connect() on " << mSession->id() << ": " << x.what();
- mSession->destroy(AsteriskSCF::Operations::createContext());
- }
- catch (const Ice::SocketException& x)
- {
- //
- // A possible retry scenario.
- //
- mLogger(Warning) << "exception when calling connect() on " << mSession->id() << ": " << x.what();
+ assert("We should not be rethrowing or ignorning ONEs" == 0);
}
catch (const std::exception& x)
{
@@ -224,6 +210,7 @@ protected:
}
catch (...)
{
+ assert(false);
mListener->failed();
}
}
diff --git a/test/TestBridging.cpp b/test/TestBridging.cpp
index 3d9bdb1..63ae68e 100644
--- a/test/TestBridging.cpp
+++ b/test/TestBridging.cpp
@@ -463,7 +463,8 @@ public:
{
AsteriskSCF::Core::Endpoint::V1::EndpointSeq eps = mLocatorPrx->lookup(id);
assert(eps.size() > 0 && eps[0] != 0);
- AsteriskSCF::SessionCommunications::V1::SessionEndpointPrx proxy(AsteriskSCF::SessionCommunications::V1::SessionEndpointPrx::checkedCast(eps[0]));
+ AsteriskSCF::SessionCommunications::V1::SessionEndpointPrx proxy(
+ AsteriskSCF::SessionCommunications::V1::SessionEndpointPrx::checkedCast(eps[0]));
assert(proxy);
AsteriskSCF::SessionCommunications::V1::SessionPrx session(
proxy->createSession(AsteriskSCF::Operations::createContext(), id, 0, 0));
-----------------------------------------------------------------------
--
asterisk-scf/integration/bridging.git
More information about the asterisk-scf-commits
mailing list