Fixed deadlock: outgoing jabber stream id is now checked by the engine event processor to avoid locking engine's stream list(s) while the stream is locked.

git-svn-id: http://yate.null.ro/svn/yate/trunk@3126 acf43c95-373e-0410-b603-e72c3f656dc1

libs/yjabber/jbengine.cpp

@@ -1022,7 +1022,7 @@ void JBEngine::buildDialbackKey(const String& id, const String& local,
 }
 
 // Check for duplicate stream id at a remote server
-bool JBEngine::checkDupId(const JBStream* stream)
+bool JBEngine::checkDupId(JBStream* stream)
 {
     if (!stream || stream->incoming())
 	return false;
@@ -1030,6 +1030,10 @@ bool JBEngine::checkDupId(const JBStream* stream)
     getStreamList(list,stream->type());
     if (!list)
 	return false;
+    stream->lock();
+    String domain = stream->remote().domain();
+    String id = stream->id();
+    stream->unlock();
     list->lock();
     JBStream* found = 0;
     for (ObjList* o = list->sets().skipNull(); o; o = o->skipNext()) {
@@ -1040,8 +1044,8 @@ bool JBEngine::checkDupId(const JBStream* stream)
 		// Lock the stream: its data might change
 		Lock lock(found);
 		// Ignore destroying streams
-		if (found->remote() == stream->remote() &&
-		    found->id() == stream->id() && found->state() != JBStream::Destroy)
+		if (found->remote().domain() == domain &&
+		    found->id() == id && found->state() != JBStream::Destroy)
 		    break;
 	    }
 	    found = 0;

libs/yjabber/jbstream.cpp

@@ -208,7 +208,8 @@ void JBStream::connectTerminated(Socket*& sock)
 	    resetConnection(sock);
 	    sock = 0;
 	    changeState(Starting);
-	    start();
+	    XmlElement* s = buildStreamStart();
+	    sendStreamXml(WaitStart,s);
 	}
 	else {
 	    DDebug(this,DebugNote,"Connect failed [%p]",this);
@@ -476,17 +477,39 @@ bool JBStream::sendStreamXml(State newState, XmlElement* first, XmlElement* seco
 }
 
 // Start the stream. This method should be called by the upper layer
-//  when processing an incoming stream Start event. For outgoing streams
-//  this method is called internally on succesfully connect.
+//  when processing an incoming stream Start event
 void JBStream::start(XMPPFeatureList* features, XmlElement* caps, bool useVer1)
 {
     Lock lock(this);
     if (m_state != Starting)
 	return;
     if (outgoing()) {
+	TelEngine::destruct(features);
 	TelEngine::destruct(caps);
-	XmlElement* s = buildStreamStart();
-	sendStreamXml(WaitStart,s);
+	if (m_type == c2s) {
+	    // c2s: just wait for stream features
+	    changeState(Features);
+	}
+	else if (m_type == s2s) {
+	    // Wait features ?
+	    if (flag(StreamRemoteVer1)) {
+		changeState(Features);
+		return;
+	    }
+	    // Stream not secured
+	    if (!flag(StreamSecured)) {
+		// Accept dialback auth stream
+		// The namspace presence was already checked in checkStreamStart()
+		if (flag(TlsRequired)) {
+		    terminate(0,false,0,XMPPError::EncryptionRequired);
+		    return;
+		}
+	    }
+	    setFlags(StreamSecured);
+	    serverStream()->sendDialback();
+	}
+	else
+	    DDebug(this,DebugStub,"JBStream::start() not handled for type=%s",typeName());
 	return;
     }
     m_features.clear();
@@ -1123,13 +1146,10 @@ bool JBStream::processStreamStart(const XmlElement* xml)
 	}
 	else {
 	    m_id = xml->getAttribute("id");
-	    if (!m_id)
-		reason = "Missing stream id";
-	    else if (m_engine->checkDupId(this))
-		reason = "Duplicate stream id";
-	    if (reason) {
+	    if (!m_id) {
 		Debug(this,DebugNote,"Received '%s' with invalid stream id='%s' [%p]",
 		    xml->tag(),m_id.c_str(),this);
+		reason = "Missing stream id";
 		error = XMPPError::InvalidId;
 		break;
 	    }
@@ -2141,15 +2161,10 @@ bool JBClientStream::processStart(const XmlElement* xml, const JabberID& from,
 	    "Invalid 'to' attribute");
 	return false;
     }
-    if (incoming()) {
+    if (incoming() || flag(StreamRemoteVer1)) {
 	m_events.append(new JBEvent(JBEvent::Start,this,0,from,to));
 	return true;
     }
-    // Wait features ?
-    if (flag(StreamRemoteVer1)) {
-	changeState(Features);
-	return true;
-    }
     Debug(this,DebugStub,"Outgoing client stream: unsupported remote version (expecting 1.x)");
     terminate(0,true,0,XMPPError::Internal,"Unsupported version");
     return false;
@@ -2640,22 +2655,8 @@ bool JBServerStream::processStart(const XmlElement* xml, const JabberID& from,
     }
 
     if (outgoing()) {
-	// Wait features ?
-	if (flag(StreamRemoteVer1)) {
-	    changeState(Features);
-	    return true;
-	}
-	// Stream not secured
-	if (!flag(StreamSecured)) {
-	    // Accept dialback auth stream
-	    // The namspace presence was already checked in checkStreamStart()
-	    if (flag(TlsRequired)) {
-		terminate(0,false,0,XMPPError::EncryptionRequired);
-		return false;
-	    }
-	}
-	setFlags(StreamSecured);
-	return sendDialback();
+	m_events.append(new JBEvent(JBEvent::Start,this,0,from,to));
+	return true;
     }
 
     // Incoming stream

libs/yjabber/yatejabber.h

@@ -744,8 +744,7 @@ public:
 
     /**
      * Start the stream. This method should be called by the upper layer
-     *  when processing an incoming stream Start event. For outgoing streams
-     *  this method is called internally on succesfully connect.
+     *  when processing an incoming stream Start event
      * This method is thread safe
      * @param features Optional features to advertise to the remote party of an
      *  incoming stream. The caller is responsable of freeing it.
@@ -1848,7 +1847,7 @@ public:
      * @param stream The calling stream
      * @return True if a duplicate is found
      */
-    bool checkDupId(const JBStream* stream);
+    bool checkDupId(JBStream* stream);
 
     /**
      * Print XML to output

modules/client/jabberclient.cpp

@@ -583,6 +583,14 @@ void YJBEngine::processEvent(JBEvent* ev)
 	    if (ev->element())
 		processRegisterEvent(ev,ev->type() == JBEvent::RegisterOk);
 	    break;
+	case JBEvent::Start:
+	    if (ev->stream()->outgoing()) {
+		if (!checkDupId(ev->stream()))
+		    ev->stream()->start();
+		else
+		    ev->stream()->terminate(-1,true,0,XMPPError::InvalidId,"Duplicate stream id");
+		break;
+	    }
 	default:
 	    returnEvent(ev,XMPPError::ServiceUnavailable);
 	    return;

modules/jabber/jabberserver.cpp

@@ -942,6 +942,12 @@ void YJBEngine::processEvent(JBEvent* ev)
 	case JBEvent::Start:
 	    if (ev->stream()->incoming())
 		processStartIn(ev);
+	    else {
+		if (!checkDupId(ev->stream()))
+		    ev->stream()->start();
+		else
+		    ev->stream()->terminate(-1,true,0,XMPPError::InvalidId,"Duplicate stream id");
+	    }
 	    break;
 	case JBEvent::Auth:
 	    if (ev->stream()->incoming())