Fix some LLCP threading issues.

On Android we now start connecting SNEP/other LLCP
services as soon as the link comes up. That means
we will have connect requests incoming, accept
responses outgoing, connect requests outgoing and
accept responses incoming.

This lays bare a lot of threading issues in this lib.
Since the receives come in serialized, those typically
do not cause any issue. However, we may be sending
something on a thread coming from the NfcService,
while at the same time we'll receive for example
a connect frame from the remote, to which the receive
thread immediately wants to send a response.

This is a first attempt at making sends thread-safe:
already there was a lot of logic to deal with the fact
that another send was in progress, in the form of the
bSendPending variable. That variable was however
not checked atomically, and some operations did not
even set it correctly.

This change tests and sets that variable atomically,
more or less implementing a semaphore with try-acquires
and fallbacks for the failure case.

The generic structure is:

if (testAndSetSendPending()) {
   // Another thread is sending, stash data
   // and most importantly do *NOT* change shared data
} else {
   // We're now the only one sending, we're free
   // to change shared data until the callback is
   // called.
}

This is just a band-aid fix, but given the fact that
this stack will be deprecated it is hopefully enough
for our planned usecases.

A decent fix would involve switching to
a TX-queue per socket, and a generic TX-queue
for the whole LLCP link. This allows us to fully
decouple the connection layer from the transport
layer, as it should be.

Change-Id: Ibd8742f7350a58459771f5036a049af3a487f783

1 files changed, 43 insertions, 42 deletions

diff --git a/src/phFriNfc_LlcpTransport.c b/src/phFriNfc_LlcpTransport.c
index 870aeb4..b3aee54 100644
--- a/src/phFriNfc_LlcpTransport.c
+++ b/src/phFriNfc_LlcpTransport.c
@@ -23,6 +23,7 @@
  */
 
 /*include files*/
+#include <cutils/log.h>
 #include <phOsalNfc.h>
 #include <phLibNfcStatus.h>
 #include <phLibNfc.h>
@@ -256,7 +257,7 @@ static NFCSTATUS phFriNfc_LlcpTransport_DiscoveryAnswer(phFriNfc_LlcpTransport_t
    uint8_t           nTid, nSap;
 
    /* Test if a send is pending */
-   if(!psTransport->bSendPending)
+   if(!testAndSetSendPending(psTransport))
    {
       /* Set the header */
       psTransport->sLlcpHeader.dsap  = PHFRINFC_LLCP_SAP_SDP;
@@ -293,9 +294,6 @@ static NFCSTATUS phFriNfc_LlcpTransport_DiscoveryAnswer(phFriNfc_LlcpTransport_t
       /* Update buffer length to match real TLV size */
       sInfoBuffer.length = nTlvOffset;
 
-      /* Send Pending */
-      psTransport->bSendPending = TRUE;
-
       /* Send SNL frame */
       result =  phFriNfc_Llcp_Send(psTransport->pLlcp,
                                    &psTransport->sLlcpHeader,
@@ -530,6 +528,20 @@ static void phFriNfc_LlcpTransport__Recv_CB(void            *pContext,
    }
 }
 
+bool_t testAndSetSendPending(phFriNfc_LlcpTransport_t* transport) {
+    bool_t currentValue;
+    pthread_mutex_lock(&transport->mutex);
+    currentValue = transport->bSendPending;
+    transport->bSendPending = TRUE;
+    pthread_mutex_unlock(&transport->mutex);
+    return currentValue;
+}
+
+void clearSendPending(phFriNfc_LlcpTransport_t* transport) {
+    pthread_mutex_lock(&transport->mutex);
+    transport->bSendPending = FALSE;
+    pthread_mutex_unlock(&transport->mutex);
+}
 
 /* TODO: comment function Transport recv CB */
 static void phFriNfc_LlcpTransport_Send_CB(void            *pContext,
@@ -538,35 +550,40 @@ static void phFriNfc_LlcpTransport_Send_CB(void            *pContext,
    phFriNfc_LlcpTransport_t         *psTransport = (phFriNfc_LlcpTransport_t*)pContext;
    NFCSTATUS                        result = NFCSTATUS_FAILED;
    phNfc_sData_t                    sFrmrBuffer;
-   phFriNfc_Llcp_Send_CB_t          pfSavedCb;
+   phFriNfc_Llcp_LinkSend_CB_t      pfSavedCb;
    void                             *pSavedContext;
    phFriNfc_LlcpTransport_Socket_t  *pCurrentSocket = NULL;
    uint8_t                          index;
 
+   // Store callbacks and socket index, so they can safely be
+   // overwritten by any code in the callback itself.
+   pfSavedCb = psTransport->pfLinkSendCb;
+   pSavedContext = psTransport->pLinkSendContext;
+   psTransport->pfLinkSendCb = NULL;
+   psTransport->pLinkSendContext = NULL;
+   index = psTransport->socketIndex;
+
    /* 1 - Reset the FLAG send pending*/
-   psTransport->bSendPending = FALSE;
+   clearSendPending(psTransport);
 
    /* 2 - Handle pending error responses */
-
    if(psTransport->bFrmrPending)
    {
-      /* Reset FRMR pending */
-      psTransport->bFrmrPending = FALSE;
-
-      /* Send Frmr */
-      sFrmrBuffer.buffer = psTransport->FrmrInfoBuffer;
-      sFrmrBuffer.length = 0x04; /* Size of FRMR Information field */
+      if (!testAndSetSendPending(psTransport)) {
+         /* Reset FRMR pending */
+         psTransport->bFrmrPending = FALSE;
 
-      /* Send Pending */
-      psTransport->bSendPending = TRUE;
+         /* Send Frmr */
+         sFrmrBuffer.buffer = psTransport->FrmrInfoBuffer;
+         sFrmrBuffer.length = 0x04; /* Size of FRMR Information field */
 
-      result =  phFriNfc_Llcp_Send(psTransport->pLlcp,
+         result =  phFriNfc_Llcp_Send(psTransport->pLlcp,
                                    &psTransport->sLlcpHeader,
                                    NULL,
                                    &sFrmrBuffer,
                                    phFriNfc_LlcpTransport_Send_CB,
                                    psTransport);
-
+      }
    }
    else if(psTransport->bDmPending)
    {
@@ -581,18 +598,12 @@ static void phFriNfc_LlcpTransport_Send_CB(void            *pContext,
    }
 
    /* 3 - Call the original callback */
-
-   if (psTransport->pfLinkSendCb != NULL)
+   if (pfSavedCb != NULL)
    {
-      pfSavedCb = psTransport->pfLinkSendCb;
-      pSavedContext = psTransport->pLinkSendContext;
-
-      psTransport->pfLinkSendCb = NULL;
-      psTransport->pLinkSendContext = NULL;
-
-      (*pfSavedCb)(pSavedContext, status);
+      (*pfSavedCb)(pSavedContext, index, status);
    }
 
+
    /* 4 - Handle pending send operations */
 
    /* Check for pending discovery requests/responses */
@@ -634,9 +645,6 @@ static void phFriNfc_LlcpTransport_Send_CB(void            *pContext,
       }
 
    } while(index != psTransport->socketIndex);
-
-   /* Save the new index */
-   psTransport->socketIndex = index;
 }
 
 
@@ -796,7 +804,8 @@ NFCSTATUS phFriNfc_LlcpTransport_LinkSend( phFriNfc_LlcpTransport_t         *Llc
                                            phFriNfc_Llcp_sPacketHeader_t    *psHeader,
                                            phFriNfc_Llcp_sPacketSequence_t  *psSequence,
                                            phNfc_sData_t                    *psInfo,
-                                           phFriNfc_Llcp_Send_CB_t          pfSend_CB,
+                                           phFriNfc_Llcp_LinkSend_CB_t      pfSend_CB,
+                                           uint8_t                          socketIndex,
                                            void                             *pContext )
 {
    NFCSTATUS status;
@@ -808,6 +817,7 @@ NFCSTATUS phFriNfc_LlcpTransport_LinkSend( phFriNfc_LlcpTransport_t         *Llc
    /* Save callback details */
    LlcpTransport->pfLinkSendCb = pfSend_CB;
    LlcpTransport->pLinkSendContext = pContext;
+   LlcpTransport->socketIndex = socketIndex;
 
    /* Call the link-level send function */
    status = phFriNfc_Llcp_Send(LlcpTransport->pLlcp, psHeader, psSequence, psInfo, phFriNfc_LlcpTransport_Send_CB, (void*)LlcpTransport);
@@ -885,7 +895,7 @@ NFCSTATUS phFriNfc_LlcpTransport_SendFrameReject(phFriNfc_LlcpTransport_t
       psTransport->FrmrInfoBuffer[3] = (vsa<<4)|vra ;
 
       /* Test if a send is pending */
-      if(psTransport->bSendPending)
+      if(testAndSetSendPending(psTransport))
       {
          psTransport->bFrmrPending = TRUE;
          status = NFCSTATUS_PENDING;
@@ -895,9 +905,6 @@ NFCSTATUS phFriNfc_LlcpTransport_SendFrameReject(phFriNfc_LlcpTransport_t
          sFrmrBuffer.buffer =  psTransport->FrmrInfoBuffer;
          sFrmrBuffer.length =  0x04; /* Size of FRMR Information field */
 
-         /* Send Pending */
-         psTransport->bSendPending = TRUE;
-
          /* Send FRMR frame */
          status =  phFriNfc_Llcp_Send(psTransport->pLlcp,
                                       &psTransport->sLlcpHeader,
@@ -927,7 +934,7 @@ NFCSTATUS phFriNfc_LlcpTransport_SendDisconnectMode(phFriNfc_LlcpTransport_t* ps
    NFCSTATUS                       status = NFCSTATUS_SUCCESS;
 
    /* Test if a send is pending */
-   if(psTransport->bSendPending)
+   if(testAndSetSendPending(psTransport))
    {
       /* DM pending */
       psTransport->bDmPending        = TRUE;
@@ -951,9 +958,6 @@ NFCSTATUS phFriNfc_LlcpTransport_SendDisconnectMode(phFriNfc_LlcpTransport_t* ps
       psTransport->sDmPayload.buffer    = &psTransport->DmInfoBuffer[2];
       psTransport->sDmPayload.length    = PHFRINFC_LLCP_DM_LENGTH;
 
-      /* Send Pending */
-      psTransport->bSendPending = TRUE;
-
       /* Send DM frame */
       status =  phFriNfc_Llcp_Send(psTransport->pLlcp,
                                    &psTransport->sDmHeader,
@@ -1075,7 +1079,7 @@ static NFCSTATUS phFriNfc_LlcpTransport_DiscoverServicesEx(phFriNfc_LlcpTranspor
    uint32_t          nTlvOffset;
 
    /* Test if a send is pending */
-   if(!psTransport->bSendPending)
+   if(!testAndSetSendPending(psTransport))
    {
       /* Set the header */
       psTransport->sLlcpHeader.dsap  = PHFRINFC_LLCP_SAP_SDP;
@@ -1111,9 +1115,6 @@ static NFCSTATUS phFriNfc_LlcpTransport_DiscoverServicesEx(phFriNfc_LlcpTranspor
       /* Update buffer length to match real TLV size */
       sInfoBuffer.length = nTlvOffset;
 
-      /* Send Pending */
-      psTransport->bSendPending = TRUE;
-
       /* Send SNL frame */
       result =  phFriNfc_Llcp_Send(psTransport->pLlcp,
                                    &psTransport->sLlcpHeader,