Ticket #759: Problem with TCP transport on Symbian

 - backported changes in #758


git-svn-id: http://svn.pjsip.org/repos/pjproject/branches/1.0@2804 74dad513-b988-da41-8d7b-12977e46ad98

6 files changed, 113 insertions, 45 deletions

diff --git a/pjlib/src/pj/ioqueue_symbian.cpp b/pjlib/src/pj/ioqueue_symbian.cpp
index ee6d7e0d..d8e3aeeb 100644
--- a/pjlib/src/pj/ioqueue_symbian.cpp
+++ b/pjlib/src/pj/ioqueue_symbian.cpp
@@ -213,7 +213,16 @@ pj_status_t CIoqueueCallback::StartRead(pj_ioqueue_op_key_t *op_key,
     } else {
 	aAddress_.SetAddress(0);
 	aAddress_.SetPort(0);
-	sock_->Socket().Recv(aBufferPtr_, flags, iStatus);
+
+	if (sock_->IsDatagram()) {
+	    sock_->Socket().Recv(aBufferPtr_, flags, iStatus);
+	} else {
+	    // Using static like this is not pretty, but we don't need to use
+	    // the value anyway, hence doing it like this is probably most
+	    // optimal.
+	    static TSockXfrLength len;
+	    sock_->Socket().RecvOneOrMore(aBufferPtr_, flags, iStatus, len);
+	}
     }
 
     SetActive();
@@ -277,6 +286,7 @@ void CIoqueueCallback::HandleReadCompletion()
 CPjSocket *CIoqueueCallback::HandleAcceptCompletion() 
 {
 	CPjSocket *pjNewSock = new CPjSocket(get_pj_socket()->GetAf(), 
+					     get_pj_socket()->GetSockType(),
 					     blank_sock_);
 	int addrlen = 0;
 	
diff --git a/pjlib/src/pj/os_symbian.h b/pjlib/src/pj/os_symbian.h
index 3497e7af..c6f2fe77 100644
--- a/pjlib/src/pj/os_symbian.h
+++ b/pjlib/src/pj/os_symbian.h
@@ -54,8 +54,9 @@ public:
     };
 
     // Construct CPjSocket
-    CPjSocket(int af, RSocket &sock)
-	: af_(af), sock_(sock), connected_(false), sockReader_(NULL)
+    CPjSocket(int af, int sock_type, RSocket &sock)
+	: af_(af), sock_(sock), sock_type_(sock_type), connected_(false), 
+	  sockReader_(NULL)
     { 
     }
 
@@ -86,6 +87,18 @@ public:
 	connected_ = connected;
     }
 
+    // Get socket type
+    int GetSockType() const
+    {
+	return sock_type_;
+    }
+    
+    // Returns true if socket is a datagram
+    bool IsDatagram() const
+    {
+	return sock_type_ == KSockDatagram;
+    }
+    
     // Get socket reader, if any.
     // May return NULL.
     CPjSocketReader *Reader()
@@ -103,6 +116,8 @@ private:
     int		     af_;
     RSocket	     sock_;	    // Must not be reference, or otherwise
 				    // it may point to local variable!
+    unsigned   	     sock_type_;
+    
     bool	     connected_;
     CPjSocketReader *sockReader_;
 };
diff --git a/pjlib/src/pj/sock_symbian.cpp b/pjlib/src/pj/sock_symbian.cpp
index a0ca80e0..60621724 100644
--- a/pjlib/src/pj/sock_symbian.cpp
+++ b/pjlib/src/pj/sock_symbian.cpp
@@ -125,13 +125,7 @@ CPjSocketReader::CPjSocketReader(CPjSocket &sock)
 
 void CPjSocketReader::ConstructL(unsigned max_len)
 {
-    TProtocolDesc aProtocol;
-    TInt err;
-
-    err = sock_.Socket().Info(aProtocol);
-    User::LeaveIfError(err);
-
-    isDatagram_ = (aProtocol.iSockType == KSockDatagram);
+    isDatagram_ = sock_.IsDatagram();
 
     TUint8 *ptr = new TUint8[max_len];
     buffer_.Set(ptr, 0, (TInt)max_len);
@@ -517,7 +511,7 @@ PJ_DEF(pj_status_t) pj_sock_socket(int af,
 
 
     /* Wrap Symbian RSocket into PJLIB's CPjSocket, and return to caller */
-    CPjSocket *pjSock = new CPjSocket(af, rSock);
+    CPjSocket *pjSock = new CPjSocket(af, type, rSock);
     *p_sock = (pj_sock_t)pjSock;
 
     return PJ_SUCCESS;
@@ -733,7 +727,6 @@ PJ_DEF(pj_status_t) pj_sock_recv(pj_sock_t sock,
     PJ_SYMBIAN_CHECK_CONNECTION();
 
     CPjSocket *pjSock = (CPjSocket*)sock;
-    RSocket &rSock = pjSock->Socket();
 
     if (pjSock->Reader()) {
 	CPjSocketReader *reader = pjSock->Reader();
@@ -757,7 +750,15 @@ PJ_DEF(pj_status_t) pj_sock_recv(pj_sock_t sock,
     TSockXfrLength recvLen;
     TPtr8 data((TUint8*)buf, (TInt)*len, (TInt)*len);
 
-    rSock.Recv(data, flags, reqStatus, recvLen);
+    if (pjSock->IsDatagram()) {
+	pjSock->Socket().Recv(data, flags, reqStatus);
+    } else {
+	// Using static like this is not pretty, but we don't need to use
+	// the value anyway, hence doing it like this is probably most
+	// optimal.
+	static TSockXfrLength len;
+	pjSock->Socket().RecvOneOrMore(data, flags, reqStatus, len);
+    }
     User::WaitForRequest(reqStatus);
 
     if (reqStatus == KErrNone) {
@@ -997,7 +998,8 @@ PJ_DEF(pj_status_t) pj_sock_accept( pj_sock_t serverfd,
     }
 
     // Create PJ socket
-    CPjSocket *newPjSock = new CPjSocket(pjSock->GetAf(), newSock);
+    CPjSocket *newPjSock = new CPjSocket(pjSock->GetAf(), pjSock->GetSockType(),
+					 newSock);
     newPjSock->SetConnected(true);
 
     *newsock = (pj_sock_t) newPjSock;
diff --git a/pjlib/src/pjlib-test/activesock.c b/pjlib/src/pjlib-test/activesock.c
index 0d1d3a22..9390d917 100644
--- a/pjlib/src/pjlib-test/activesock.c
+++ b/pjlib/src/pjlib-test/activesock.c
@@ -212,7 +212,11 @@ static int udp_ping_pong_test(void)
 
 	for (i=0; i<10 && last_rx1 == srv1->rx_cnt && last_rx2 == srv2->rx_cnt; ++i) {
 	    pj_time_val delay = {0, 10};
+#ifdef PJ_SYMBIAN
+	    pj_symbianos_poll(-1, 100);
+#else
 	    pj_ioqueue_poll(ioqueue, &delay);
+#endif
 	}
 
 	if (srv1->rx_err_cnt+srv1->tx_err_cnt != 0 ||
@@ -403,26 +407,47 @@ static int tcp_perf_test(void)
 	status = pj_activesock_send(asock2, &op_key->op_key, pkt, &len, 0);
 	if (status == PJ_EPENDING) {
 	    do {
+#if PJ_SYMBIAN
+		pj_symbianos_poll(-1, -1);
+#else
 		pj_ioqueue_poll(ioqueue, NULL);
+#endif
 	    } while (!state2->sent);
-	} else if (status != PJ_SUCCESS) {
-		PJ_LOG(1,("", "   err: send status=%d", status));
-	    status = -180;
-	    break;
-	} else if (status == PJ_SUCCESS) {
-	    if (len != sizeof(*pkt)) {
-		PJ_LOG(1,("", "   err: shouldn't report partial sent"));
-		status = -190;
-		break;
-	    }
+	} else {
+#if PJ_SYMBIAN
+		/* The Symbian socket always returns PJ_SUCCESS for TCP send,
+		 * eventhough the remote end hasn't received the data yet.
+		 * If we continue sending, eventually send() will block,
+		 * possibly because the send buffer is full. So we need to
+		 * poll the ioqueue periodically, to let receiver gets the 
+		 * data.
+		 */
+		pj_symbianos_poll(-1, 0);
+#endif
+		if (status != PJ_SUCCESS) {
+		    PJ_LOG(1,("", "   err: send status=%d", status));
+		    status = -180;
+		    break;
+		} else if (status == PJ_SUCCESS) {
+		    if (len != sizeof(*pkt)) {
+			PJ_LOG(1,("", "   err: shouldn't report partial sent"));
+			status = -190;
+			break;
+		    }
+		}
 	}
     }
 
     /* Wait until everything has been sent/received */
     if (state1->next_recv_seq < COUNT) {
+#ifdef PJ_SYMBIAN
+	while (pj_symbianos_poll(-1, 1000) == PJ_TRUE)
+	    ;
+#else
 	pj_time_val delay = {0, 100};
 	while (pj_ioqueue_poll(ioqueue, &delay) > 0)
 	    ;
+#endif
     }
 
     if (status == PJ_EPENDING)
diff --git a/pjsip-apps/src/symbian_ua/ua.cpp b/pjsip-apps/src/symbian_ua/ua.cpp
index 11adb154..d48fb15a 100644
--- a/pjsip-apps/src/symbian_ua/ua.cpp
+++ b/pjsip-apps/src/symbian_ua/ua.cpp
@@ -49,6 +49,10 @@
 #define SIP_PROXY	NULL
 //#define SIP_PROXY	"<sip:192.168.0.8;lr>"
 
+//
+// Set to 1 if TCP is desired (experimental)
+//
+#define ENABLE_SIP_TCP	0
 
 //
 // Configure nameserver if DNS SRV is to be used with both SIP
@@ -379,11 +383,23 @@ static pj_status_t app_startup()
     tcfg.port = SIP_PORT;
     status = pjsua_transport_create(PJSIP_TRANSPORT_UDP, &tcfg, &tid);
     if (status != PJ_SUCCESS) {
-	    pjsua_perror(THIS_FILE, "Error creating transport", status);
+	    pjsua_perror(THIS_FILE, "Error creating UDP transport", status);
 	    pjsua_destroy();
 	    return status;
     }
 
+    /* Add TCP transport */
+#if ENABLE_SIP_TCP
+    pjsua_transport_config_default(&tcfg);
+    tcfg.port = SIP_PORT;
+    status = pjsua_transport_create(PJSIP_TRANSPORT_TCP, &tcfg, &tid);
+    if (status != PJ_SUCCESS) {
+	    pjsua_perror(THIS_FILE, "Error creating TCP transport", status);
+	    pjsua_destroy();
+	    return status;
+    }
+#endif
+    
     /* Add account for the transport */
     pjsua_acc_add_local(tid, PJ_TRUE, &g_acc_id);
 
diff --git a/pjsip/src/pjsip/sip_transport_tcp.c b/pjsip/src/pjsip/sip_transport_tcp.c
index a9b3bd27..1692459a 100644
--- a/pjsip/src/pjsip/sip_transport_tcp.c
+++ b/pjsip/src/pjsip/sip_transport_tcp.c
@@ -805,33 +805,33 @@ static pj_status_t lis_create_transport(pjsip_tpfactory *factory,
     status = pj_activesock_start_connect(tcp->asock, tcp->base.pool, rem_addr,
 					 sizeof(pj_sockaddr_in));
     if (status == PJ_SUCCESS) {
-	tcp->has_pending_connect = PJ_FALSE;
+	on_connect_complete(tcp->asock, PJ_SUCCESS);
     } else if (status != PJ_EPENDING) {
 	tcp_destroy(&tcp->base, status);
 	return status;
     }
 
-    /* Update (again) local address, just in case local address currently
-     * set is different now that asynchronous connect() is started.
-     */
-    addr_len = sizeof(pj_sockaddr_in);
-    if (pj_sock_getsockname(sock, &local_addr, &addr_len)==PJ_SUCCESS) {
-	pj_sockaddr_in *tp_addr = (pj_sockaddr_in*)&tcp->base.local_addr;
-
-	/* Some systems (like old Win32 perhaps) may not set local address
-	 * properly before socket is fully connected.
+    if (tcp->has_pending_connect) {
+	/* Update (again) local address, just in case local address currently
+	 * set is different now that asynchronous connect() is started.
 	 */
-	if (tp_addr->sin_addr.s_addr != local_addr.sin_addr.s_addr &&
-	    local_addr.sin_addr.s_addr != 0) 
-	{
-	    tp_addr->sin_addr.s_addr = local_addr.sin_addr.s_addr;
-	    tp_addr->sin_port = local_addr.sin_port;
-	    sockaddr_to_host_port(tcp->base.pool, &tcp->base.local_name,
-				  &local_addr);
-	}
-    }
+	addr_len = sizeof(pj_sockaddr_in);
+	if (pj_sock_getsockname(sock, &local_addr, &addr_len)==PJ_SUCCESS) {
+	    pj_sockaddr_in *tp_addr = (pj_sockaddr_in*)&tcp->base.local_addr;
 
-    if (tcp->has_pending_connect) {
+	    /* Some systems (like old Win32 perhaps) may not set local address
+	     * properly before socket is fully connected.
+	     */
+	    if (tp_addr->sin_addr.s_addr != local_addr.sin_addr.s_addr &&
+		local_addr.sin_addr.s_addr != 0) 
+	    {
+		tp_addr->sin_addr.s_addr = local_addr.sin_addr.s_addr;
+		tp_addr->sin_port = local_addr.sin_port;
+		sockaddr_to_host_port(tcp->base.pool, &tcp->base.local_name,
+				      &local_addr);
+	    }
+	}
+	
 	PJ_LOG(4,(tcp->base.obj_name, 
 		  "TCP transport %.*s:%d is connecting to %.*s:%d...",
 		  (int)tcp->base.local_name.host.slen,