- MMS server: fixed - possible deadlock in obtainFile-service/file upload task (LIB61850-351)

src/mms/inc_private/mms_server_internal.h

@@ -219,7 +219,7 @@ LIB61850_INTERNAL MmsObtainFileTask
 MmsServer_getObtainFileTask(MmsServer self);
 
 LIB61850_INTERNAL void
-mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task);
+mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task, int taskState);
 #endif
 
 LIB61850_INTERNAL ByteBuffer*

src/mms/iso_mms/server/mms_file_service.c

@@ -411,8 +411,29 @@ createObtainFileResponse(uint32_t invokeId, ByteBuffer* response)
 }
 
 void
-mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
+mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task, int taskState)
 {
+    /* call locks in certain order (lock IsoConnection -> reserverTransmitBuffer, task->taskLock) to prevent potential deadlock */
+
+    ByteBuffer* message = NULL;
+
+    if (taskState == MMS_FILE_UPLOAD_STATE_SEND_FILE_READ || 
+        taskState == MMS_FILE_UPLOAD_STATE_SEND_FILE_CLOSE ||
+        taskState == MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE ||
+        taskState == MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_DESTINATION ||
+        taskState == MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_RESPONSE) 
+    {
+        IsoConnection_lock(task->connection->isoConnection);
+
+        message = MmsServer_reserveTransmitBuffer(self);
+    }
+
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    Semaphore_wait(task->taskLock);
+#endif
+
+    if (task->state == taskState) {
+
         switch (task->state) {
 
             case MMS_FILE_UPLOAD_STATE_NOT_USED:
@@ -438,20 +459,12 @@ 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);
+                    mmsClient_createFileReadRequest(task->lastRequestInvokeId, message, task->frmsId);
 
                     task->state = MMS_FILE_UPLOAD_STATE_FILE_READ_SENT;
-                IsoConnection_sendMessage(task->connection->isoConnection, request);
+                    IsoConnection_sendMessage(task->connection->isoConnection, message);
-
-                MmsServer_releaseTransmitBuffer(self);
-
-                IsoConnection_unlock(task->connection->isoConnection);
 
                     task->nextTimeout = Hal_getTimeInMs() + 2000; /* timeout 2000 ms */
                 }
@@ -478,21 +491,13 @@ 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);
+                    mmsClient_createFileCloseRequest(task->lastRequestInvokeId, message, task->frmsId);
 
                     task->state = MMS_FILE_UPLOAD_STATE_FILE_CLOSE_SENT;
 
-                IsoConnection_sendMessage(task->connection->isoConnection, request);
+                    IsoConnection_sendMessage(task->connection->isoConnection, message);
-
-                MmsServer_releaseTransmitBuffer(self);
-
-                IsoConnection_unlock(task->connection->isoConnection);
 
                     task->nextTimeout = Hal_getTimeInMs() + 2000; /* timeout 2000 ms */
 
@@ -519,17 +524,9 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
 
                 {
                     /* send ObtainFileError */
-                IsoConnection_lock(task->connection->isoConnection);
+                    createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, message, MMS_ERROR_FILE_FILE_NON_EXISTENT, 0);
-
-                ByteBuffer* response = MmsServer_reserveTransmitBuffer(self);
-
-                createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, response, MMS_ERROR_FILE_FILE_NON_EXISTENT, 0);
 
-                IsoConnection_sendMessage(task->connection->isoConnection, response);
+                    IsoConnection_sendMessage(task->connection->isoConnection, message);
-
-                MmsServer_releaseTransmitBuffer(self);
-
-                IsoConnection_unlock(task->connection->isoConnection);
 
                     if(task->fileHandle){
                         FileSystem_closeFile(task->fileHandle);
@@ -548,17 +545,9 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
             case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_DESTINATION:
                 {
                     /* send ObtainFileError */
-                IsoConnection_lock(task->connection->isoConnection);
+                    createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, message, MMS_ERROR_FILE_OTHER, 1);
-
-                ByteBuffer* response = MmsServer_reserveTransmitBuffer(self);
 
-                createServiceErrorObtainFileError(task->obtainFileRequestInvokeId, response, MMS_ERROR_FILE_OTHER, 1);
+                    IsoConnection_sendMessage(task->connection->isoConnection, message);
-
-                IsoConnection_sendMessage(task->connection->isoConnection, response);
-
-                MmsServer_releaseTransmitBuffer(self);
-
-                IsoConnection_unlock(task->connection->isoConnection);
 
                     if (task->fileHandle) {
                         FileSystem_closeFile(task->fileHandle);
@@ -578,24 +567,17 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
 
             case MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_RESPONSE:
                 {
-                IsoConnection_lock(task->connection->isoConnection);
+                    createObtainFileResponse(task->obtainFileRequestInvokeId, message);
-
-                ByteBuffer* response = MmsServer_reserveTransmitBuffer(self);
-
-                createObtainFileResponse(task->obtainFileRequestInvokeId, response);
 
                     task->state = MMS_FILE_UPLOAD_STATE_NOT_USED;
 
-                IsoConnection_sendMessage(task->connection->isoConnection, response);
+                    IsoConnection_sendMessage(task->connection->isoConnection, message);
-
-                MmsServer_releaseTransmitBuffer(self);
-
-                IsoConnection_unlock(task->connection->isoConnection);
 
                     if (self->getFileCompleteHandler)
                         self->getFileCompleteHandler(self->getFileCompleteHandlerParameter, task->connection, task->destinationFilename);
                 }
                 break;
+
             case MMS_FILE_UPLOAD_STATE_INTERRUPTED:
                 {
                     if (DEBUG_MMS_SERVER)
@@ -612,6 +594,16 @@ mmsServer_fileUploadTask(MmsServer self, MmsObtainFileTask task)
                 }
                 break;
         }
+    }
+
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+    Semaphore_post(task->taskLock);
+#endif
+
+    if (message) {
+        MmsServer_releaseTransmitBuffer(self);
+        IsoConnection_unlock(task->connection->isoConnection);
+    }
 }
 
 #if (MMS_OBTAIN_FILE_SERVICE == 1)
@@ -770,6 +762,10 @@ mmsServer_handleObtainFileRequest(
 
                 task->state = MMS_FILE_UPLOAD_STATE_FILE_OPEN_SENT;
             }
+
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+            Semaphore_post(task->taskLock);
+#endif
         }
         else
             goto exit_unavailable;

src/mms/iso_mms/server/mms_server.c

@@ -316,10 +316,6 @@ MmsServer_getObtainFileTask(MmsServer self)
         if (self->fileUploadTasks[i].state == 0) {
             self->fileUploadTasks[i].state = 1;
 
-#if (CONFIG_MMS_THREADLESS_STACK != 1)
-            Semaphore_post(self->fileUploadTasks[i].taskLock);
-#endif
-
             return &(self->fileUploadTasks[i]);
         }
 
@@ -742,15 +738,15 @@ MmsServer_handleBackgroundTasks(MmsServer self)
         Semaphore_wait(self->fileUploadTasks[i].taskLock);
 #endif
 
-        if (self->fileUploadTasks[i].state != 0) {
+        int taskState = self->fileUploadTasks[i].state;
-
-            if (self->fileUploadTasks[i].state != 0)
-                mmsServer_fileUploadTask(self, &(self->fileUploadTasks[i]));
-        }
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
         Semaphore_post(self->fileUploadTasks[i].taskLock);
 #endif
+
+        if (taskState != 0) {
+            mmsServer_fileUploadTask(self, &(self->fileUploadTasks[i]), taskState);
+        }
     }
 
 #endif /* (MMS_OBTAIN_FILE_SERVICE == 1) */