- MMS server: fixed potential deadlock in multi-thread mode

src/iec61850/server/mms_mapping/reporting.c

@@ -349,6 +349,8 @@ sendReportSegment(ReportControl* self, bool isIntegrity, bool isGI)
     if (self->clientConnection == NULL)
         return false;
 
+    IsoConnection_lock(self->clientConnection->isoConnection);
+
     int maxMmsPduSize = MmsServerConnection_getMaxMmsPduSize(self->clientConnection);
     int estimatedSegmentSize = 19; /* maximum size of header information (header can have 13-19 byte) */
     estimatedSegmentSize += 8; /* reserve space for more-segments-follow (3 byte) and sub-seq-num (3-5 byte) */
@@ -687,10 +689,12 @@ sendReportSegment(ReportControl* self, bool isIntegrity, bool isGI)
 
     reportBuffer->size = bufPos;
 
-    MmsServerConnection_sendMessage(self->clientConnection, reportBuffer, false);
+    MmsServerConnection_sendMessage(self->clientConnection, reportBuffer);
 
     MmsServer_releaseTransmitBuffer(self->server->mmsServer);
 
+    IsoConnection_unlock(self->clientConnection->isoConnection);
+
     if (moreFollows == false) {
         /* reset sub sequence number */
         segmented = false;
@@ -2874,6 +2878,8 @@ sendNextReportEntrySegment(ReportControl* self)
 
     ReportControl_unlockNotify(self);
 
+    IsoConnection_lock(self->clientConnection->isoConnection);
+
     ByteBuffer* reportBuffer = MmsServer_reserveTransmitBuffer(self->server->mmsServer);
 
     uint8_t* buffer = reportBuffer->buffer;
@@ -3062,10 +3068,12 @@ sendNextReportEntrySegment(ReportControl* self)
 
     reportBuffer->size = bufPos;
 
-    MmsServerConnection_sendMessage(self->clientConnection, reportBuffer, false);
+    MmsServerConnection_sendMessage(self->clientConnection, reportBuffer);
 
     MmsServer_releaseTransmitBuffer(self->server->mmsServer);
 
+    IsoConnection_unlock(self->clientConnection->isoConnection);
+
     if (moreFollows == false) {
         /* reset sub sequence number */
         segmented = false;

src/mms/inc_private/iso_server.h

@@ -72,6 +72,12 @@ IsoConnection_getPeerAddress(IsoConnection self);
 LIB61850_INTERNAL void
 IsoConnection_close(IsoConnection self);
 
+LIB61850_INTERNAL void
+IsoConnection_lock(IsoConnection self);
+
+LIB61850_INTERNAL void
+IsoConnection_unlock(IsoConnection self);
+
 LIB61850_INTERNAL void
 IsoConnection_installListener(IsoConnection self, MessageReceivedHandler handler,
         void* parameter);
@@ -86,7 +92,7 @@ IsoConnection_getSecurityToken(IsoConnection self);
  *        (handlerMode)
  */
 LIB61850_INTERNAL void
-IsoConnection_sendMessage(IsoConnection self, ByteBuffer* message, bool handlerMode);
+IsoConnection_sendMessage(IsoConnection self, ByteBuffer* message);
 
 LIB61850_INTERNAL IsoServer
 IsoServer_create(TLSConfiguration tlsConfiguration);

src/mms/inc_private/mms_server_connection.h

@@ -52,7 +52,7 @@ LIB61850_INTERNAL int
 MmsServerConnection_getMaxMmsPduSize(MmsServerConnection self);
 
 LIB61850_INTERNAL void
-MmsServerConnection_sendMessage(MmsServerConnection self, ByteBuffer* message, bool handlerMode);
+MmsServerConnection_sendMessage(MmsServerConnection self, ByteBuffer* message);
 
 LIB61850_INTERNAL bool
 MmsServerConnection_addNamedVariableList(MmsServerConnection self, MmsNamedVariableList variableList);

src/mms/iso_mms/server/mms_file_service.c

@@ -434,16 +434,20 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
 
         case MMS_FILE_UPLOAD_STATE_SEND_FILE_READ:
             {
+                IsoConnection_lock(task->connection->isoConnection);
+
                 ByteBuffer* request = MmsServer_reserveTransmitBuffer(self);
 
                 task->lastRequestInvokeId = MmsServerConnection_getNextRequestInvokeId(task->connection);
 
                 mmsClient_createFileReadRequest(task->lastRequestInvokeId, request, task->frmsId);
 
-                IsoConnection_sendMessage(task->connection->isoConnection, request, false);
+                IsoConnection_sendMessage(task->connection->isoConnection, request);
 
                 MmsServer_releaseTransmitBuffer(self);
 
+                IsoConnection_unlock(task->connection->isoConnection);
+
                 task->nextTimeout = Hal_getTimeInMs() + 2000; /* timeout 2000 ms */
 
                 task->state = MMS_FILE_UPLOAD_STATE_FILE_READ_SENT;
@@ -468,16 +472,20 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
 
         case MMS_FILE_UPLOAD_STATE_SEND_FILE_CLOSE:
             {
+                IsoConnection_lock(task->connection->isoConnection);
+
                 ByteBuffer* request = MmsServer_reserveTransmitBuffer(self);
 
                 task->lastRequestInvokeId = MmsServerConnection_getNextRequestInvokeId(task->connection);
 
                 mmsClient_createFileCloseRequest(task->lastRequestInvokeId, request, task->frmsId);
 
-                IsoConnection_sendMessage(task->connection->isoConnection, request, false);
+                IsoConnection_sendMessage(task->connection->isoConnection, request);
 
                 MmsServer_releaseTransmitBuffer(self);
 
+                IsoConnection_unlock(task->connection->isoConnection);
+
                 task->nextTimeout = Hal_getTimeInMs() + 2000; /* timeout 2000 ms */
 
                 task->state = MMS_FILE_UPLOAD_STATE_FILE_CLOSE_SENT;
@@ -504,17 +512,21 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
             {
                 /* send ObtainFileError */
 
+                IsoConnection_lock(task->connection->isoConnection);
+
                 ByteBuffer* response = MmsServer_reserveTransmitBuffer(self);
 
                 createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, response, MMS_ERROR_FILE_FILE_NON_EXISTENT, 0);
 
-                IsoConnection_sendMessage(task->connection->isoConnection, response, false);
+                IsoConnection_sendMessage(task->connection->isoConnection, response);
+
+                MmsServer_releaseTransmitBuffer(self);
+
+                IsoConnection_unlock(task->connection->isoConnection);
 
                 FileSystem_closeFile(task->fileHandle);
                 deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename);
 
-                MmsServer_releaseTransmitBuffer(self);
-
                 if (DEBUG_MMS_SERVER)
                     printf("MMS_SERVER: ObtainFile service: failed to open file from client\n");
 
@@ -526,11 +538,17 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
             {
                 /* send ObtainFileError */
 
+                IsoConnection_lock(task->connection->isoConnection);
+
                 ByteBuffer* response = MmsServer_reserveTransmitBuffer(self);
 
                 createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, response, MMS_ERROR_FILE_OTHER, 1);
 
-                IsoConnection_sendMessage(task->connection->isoConnection, response, false);
+                IsoConnection_sendMessage(task->connection->isoConnection, response);
+
+                MmsServer_releaseTransmitBuffer(self);
+
+                IsoConnection_unlock(task->connection->isoConnection);
 
                 if (task->fileHandle) {
                     FileSystem_closeFile(task->fileHandle);
@@ -539,8 +557,6 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
                         deleteFile(MmsServerConnection_getFilesystemBasepath(task->connection), task->destinationFilename);
                 }
 
-                MmsServer_releaseTransmitBuffer(self);
-
                 if (DEBUG_MMS_SERVER)
                     printf("MMS_SERVER: ObtainFile service: failed to create local file\n");
 
@@ -551,14 +567,18 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
 
         case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_RESPONSE:
             {
+                IsoConnection_lock(task->connection->isoConnection);
+
                 ByteBuffer* response = MmsServer_reserveTransmitBuffer(self);
 
                 createObtainFileResponse(task->obtainFileRequestInvokeId, response);
 
-                IsoConnection_sendMessage(task->connection->isoConnection, response, false);
+                IsoConnection_sendMessage(task->connection->isoConnection, response);
 
                 MmsServer_releaseTransmitBuffer(self);
 
+                IsoConnection_unlock(task->connection->isoConnection);
+
                 if (self->getFileCompleteHandler)
                     self->getFileCompleteHandler(self->getFileCompleteHandlerParameter, task->connection, task->destinationFilename);
 
@@ -678,7 +698,7 @@ mmsServer_handleObtainFileRequest(
 
                 mmsClient_createFileOpenRequest(task->lastRequestInvokeId, request, sourceFilename, 0);
 
-                IsoConnection_sendMessage(task->connection->isoConnection, request, true);
+                IsoConnection_sendMessage(task->connection->isoConnection, request);
 
                 MmsServer_releaseTransmitBuffer(connection->server);
 

src/mms/iso_mms/server/mms_information_report.c

@@ -1,7 +1,7 @@
 /*
  *  mms_information_report.c
  *
- *  Copyright 2013, 2015 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -59,6 +59,11 @@ MmsServerConnection_sendInformationReportSingleVariableVMDSpecific(MmsServerConn
 
 	if (DEBUG_MMS_SERVER) printf("MMS_SERVER: sendInfReportSingle variable: %s\n", itemId);
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    if (handlerMode == false)
+        IsoConnection_lock(self->isoConnection);
+#endif
+
 	ByteBuffer* reportBuffer = MmsServer_reserveTransmitBuffer(self->server);
 
 	uint8_t* buffer = reportBuffer->buffer;
@@ -80,10 +85,15 @@ MmsServerConnection_sendInformationReportSingleVariableVMDSpecific(MmsServerConn
 
     reportBuffer->size = bufPos;
 
-    IsoConnection_sendMessage(self->isoConnection, reportBuffer, handlerMode);
+    IsoConnection_sendMessage(self->isoConnection, reportBuffer);
 
     MmsServer_releaseTransmitBuffer(self->server);
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    if (handlerMode == false)
+        IsoConnection_unlock(self->isoConnection);
+#endif
+
 exit_function:
     return;
 }
@@ -152,6 +162,11 @@ MmsServerConnection_sendInformationReportListOfVariables(
         goto exit_function;
     }
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    if (handlerMode == false)
+        IsoConnection_lock(self->isoConnection);
+#endif
+
     /* encode message */
     ByteBuffer* reportBuffer = MmsServer_reserveTransmitBuffer(self->server);
 
@@ -212,10 +227,16 @@ MmsServerConnection_sendInformationReportListOfVariables(
 
     reportBuffer->size = bufPos;
 
-    IsoConnection_sendMessage(self->isoConnection, reportBuffer, handlerMode);
+    IsoConnection_sendMessage(self->isoConnection, reportBuffer);
 
     MmsServer_releaseTransmitBuffer(self->server);
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    if (handlerMode == false) {
+        IsoConnection_unlock(self->isoConnection);
+    }
+#endif
+
 exit_function:
     return;
 }
@@ -265,7 +286,10 @@ MmsServerConnection_sendInformationReportVMDSpecific(MmsServerConnection self, c
         goto exit_function;
     }
 
-    if (DEBUG_MMS_SERVER) printf("MMS_SERVER: sendInfReport\n");
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    if (handlerMode == false)
+        IsoConnection_lock(self->isoConnection);
+#endif
 
     ByteBuffer* reportBuffer =  MmsServer_reserveTransmitBuffer(self->server);
 
@@ -295,10 +319,15 @@ MmsServerConnection_sendInformationReportVMDSpecific(MmsServerConnection self, c
 
     reportBuffer->size = bufPos;
 
-    IsoConnection_sendMessage(self->isoConnection, reportBuffer, false);
+    IsoConnection_sendMessage(self->isoConnection, reportBuffer);
 
     MmsServer_releaseTransmitBuffer(self->server);
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    if (handlerMode == false)
+        IsoConnection_unlock(self->isoConnection);
+#endif
+
 exit_function:
     return;
 }

src/mms/iso_mms/server/mms_server_connection.c

@@ -752,9 +752,9 @@ MmsServerConnection_getMaxMmsPduSize(MmsServerConnection self)
 }
 
 void
-MmsServerConnection_sendMessage(MmsServerConnection self, ByteBuffer* message, bool handlerMode)
+MmsServerConnection_sendMessage(MmsServerConnection self, ByteBuffer* message)
 {
-    IsoConnection_sendMessage(self->isoConnection, message, false);
+    IsoConnection_sendMessage(self->isoConnection, message);
 }
 
 #if (MMS_DYNAMIC_DATA_SETS == 1)

src/mms/iso_mms/server/mms_write_service.c

@@ -83,15 +83,21 @@ mmsServer_createMmsWriteResponse(MmsServerConnection connection,
 void
 MmsServerConnection_sendWriteResponse(MmsServerConnection self, uint32_t invokeId, MmsDataAccessError indication, bool handlerMode)
 {
+    if (handlerMode == false)
+        IsoConnection_lock(self->isoConnection);
+
     ByteBuffer* response = MmsServer_reserveTransmitBuffer(self->server);
 
     ByteBuffer_setSize(response, 0);
 
     mmsServer_createMmsWriteResponse(self, invokeId, response, 1, &indication);
 
-    IsoConnection_sendMessage(self->isoConnection, response, handlerMode);
+    IsoConnection_sendMessage(self->isoConnection, response);
 
     MmsServer_releaseTransmitBuffer(self->server);
+
+    if (handlerMode == false)
+        IsoConnection_unlock(self->isoConnection);
 }
 
 #if 0

src/mms/iso_server/iso_connection.c

@@ -163,13 +163,13 @@ IsoConnection_handleTcpConnection(IsoConnection self)
             printf("ISO_SERVER: COTP connection indication\n");
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-        Semaphore_wait(self->conMutex);
+        IsoConnection_lock(self);
 #endif
 
         CotpConnection_sendConnectionResponseMessage(self->cotpConnection);
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-        Semaphore_post(self->conMutex);
+        IsoConnection_unlock(self);
 #endif
 
         break;
@@ -200,7 +200,7 @@ IsoConnection_handleTcpConnection(IsoConnection self)
                     if (aIndication == ACSE_ASSOCIATE) {
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-                        Semaphore_wait(self->conMutex);
+                        IsoConnection_lock(self);
 #endif
 
                         if (DEBUG_ISO_SERVER)
@@ -263,7 +263,7 @@ IsoConnection_handleTcpConnection(IsoConnection self)
                         }
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-                        Semaphore_post(self->conMutex);
+                        IsoConnection_unlock(self);
 #endif
                     }
                     else {
@@ -295,7 +295,7 @@ IsoConnection_handleTcpConnection(IsoConnection self)
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
                     IsoServer_userLock(self->isoServer);
-                    Semaphore_wait(self->conMutex);
+                    IsoConnection_lock(self);
 #endif
 
                     ByteBuffer_wrap(&mmsResponseBuffer, self->sendBuffer, 0, SEND_BUF_SIZE);
@@ -334,7 +334,7 @@ IsoConnection_handleTcpConnection(IsoConnection self)
                     }
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-                    Semaphore_post(self->conMutex);
+                    IsoConnection_unlock(self);
                     IsoServer_userUnlock(self->isoServer);
 #endif
                 }
@@ -355,7 +355,7 @@ IsoConnection_handleTcpConnection(IsoConnection self)
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
                     IsoServer_userLock(self->isoServer);
-                    Semaphore_wait(self->conMutex);
+                    IsoConnection_lock(self);
 #endif
 
                     struct sBufferChain acseBufferPartStruct;
@@ -382,7 +382,7 @@ IsoConnection_handleTcpConnection(IsoConnection self)
                     CotpConnection_sendDataMessage(self->cotpConnection, sessionBufferPart);
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-                    Semaphore_post(self->conMutex);
+                    IsoConnection_unlock(self);
                     IsoServer_userUnlock(self->isoServer);
 #endif
                 }
@@ -547,7 +547,23 @@ IsoConnection_getPeerAddress(IsoConnection self)
 }
 
 void
-IsoConnection_sendMessage(IsoConnection self, ByteBuffer* message, bool handlerMode)
+IsoConnection_lock(IsoConnection self)
+{
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    Semaphore_wait(self->conMutex);
+#endif
+}
+
+void
+IsoConnection_unlock(IsoConnection self)
+{
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    Semaphore_post(self->conMutex);
+#endif
+}
+
+void
+IsoConnection_sendMessage(IsoConnection self, ByteBuffer* message)
 {
     if (self->state == ISO_CON_STATE_STOPPED) {
         if (DEBUG_ISO_SERVER)
@@ -555,15 +571,6 @@ IsoConnection_sendMessage(IsoConnection self, ByteBuffer* message, bool handlerM
         goto exit_error;
     }
 
-    bool locked = false;
-
-#if (CONFIG_MMS_THREADLESS_STACK != 1)
-    if (handlerMode == false) {
-        Semaphore_wait(self->conMutex);
-        locked = true;
-    }
-#endif
-
     struct sBufferChain payloadBufferStruct;
     BufferChain payloadBuffer = &payloadBufferStruct;
     payloadBuffer->length = message->size;
@@ -597,11 +604,6 @@ IsoConnection_sendMessage(IsoConnection self, ByteBuffer* message, bool handlerM
             printf("ISO_SERVER: IsoConnection_sendMessage success!\n");
     }
 
-#if (CONFIG_MMS_THREADLESS_STACK != 1)
-    if (locked)
-        Semaphore_post(self->conMutex);
-#endif
-
 exit_error:
     return;
 }

src/mms/iso_server/iso_server.c

@@ -83,6 +83,7 @@ struct sIsoServer {
 #endif /* (CONFIG_MAXIMUM_TCP_CLIENT_CONNECTIONS == -1) */
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
+    /* used to control access to server data model */
     Semaphore userLock;
 #endif