- fixed - potential race condition when using IedConnection_installRe…

…portHandler and IedConnection_uninstallReportHandler
mz-automation · Feb 15, 2024 · cdd0684 · cdd0684
1 parent d6c53b1
commit cdd0684
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 14 deletions.
diff --git a/src/iec61850/client/client_report.c b/src/iec61850/client/client_report.c
@@ -3,7 +3,7 @@
  *
  *  Client implementation for IEC 61850 reporting.
  *
- *  Copyright 2013-2022 Michael Zillgith
+ *  Copyright 2013-2024 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -283,14 +283,27 @@ lookupReportHandler(IedConnection self, const char* rcbReference)
     return NULL;
 }
 
+static void
+uninstallReportHandler(IedConnection self, const char* rcbReference)
+{
+    ClientReport report = lookupReportHandler(self, rcbReference);
+
+    if (report != NULL) {
+        LinkedList_remove(self->enabledReports, report);
+        ClientReport_destroy(report);
+    }
+}
+
 void
 IedConnection_installReportHandler(IedConnection self, const char* rcbReference, const char* rptId, ReportCallbackFunction handler,
         void* handlerParameter)
 {
+    Semaphore_wait(self->reportHandlerMutex);
+
     ClientReport report = lookupReportHandler(self, rcbReference);
 
     if (report != NULL) {
-        IedConnection_uninstallReportHandler(self, rcbReference);
+        uninstallReportHandler(self, rcbReference);
 
         if (DEBUG_IED_CLIENT)
             printf("DEBUG_IED_CLIENT: Removed existing report callback handler for %s\n", rcbReference);
@@ -306,8 +319,8 @@ IedConnection_installReportHandler(IedConnection self, const char* rcbReference,
     else
         report->rptId = NULL;
 
-    Semaphore_wait(self->reportHandlerMutex);
     LinkedList_add(self->enabledReports, report);
+
     Semaphore_post(self->reportHandlerMutex);
 
     if (DEBUG_IED_CLIENT)
@@ -319,12 +332,7 @@ IedConnection_uninstallReportHandler(IedConnection self, const char* rcbReferenc
 {
 	Semaphore_wait(self->reportHandlerMutex);
 
-    ClientReport report = lookupReportHandler(self, rcbReference);
-
-    if (report != NULL) {
-        LinkedList_remove(self->enabledReports, report);
-        ClientReport_destroy(report);
-    }
+    uninstallReportHandler(self, rcbReference);
 
     Semaphore_post(self->reportHandlerMutex);
 }
@@ -367,6 +375,8 @@ IedConnection_triggerGIReport(IedConnection self, IedClientError* error, const c
 void
 iedConnection_handleReport(IedConnection self, MmsValue* value)
 {
+    Semaphore_wait(self->reportHandlerMutex);
+
     MmsValue* rptIdValue = MmsValue_getElement(value, 0);
 
     if ((rptIdValue == NULL) || (MmsValue_getType(rptIdValue) != MMS_VISIBLE_STRING)) {
@@ -769,15 +779,14 @@ iedConnection_handleReport(IedConnection self, MmsValue* value)
             matchingReport->reasonForInclusion[i] = IEC61850_REASON_NOT_INCLUDED;
         }
     }
-
-    Semaphore_wait(self->reportHandlerMutex);
 
     if (matchingReport->callback != NULL)
         matchingReport->callback(matchingReport->callbackParameter, matchingReport);
 
+exit_function:
+
     Semaphore_post(self->reportHandlerMutex);
 
-exit_function:
     return;
 }
 
diff --git a/src/iec61850/inc/iec61850_client.h b/src/iec61850/inc/iec61850_client.h
@@ -1,7 +1,7 @@
 /*
  *  iec61850_client.h
  *
- *  Copyright 2013-2021 Michael Zillgith
+ *  Copyright 2013-2023 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -1256,9 +1256,11 @@ typedef void (*ReportCallbackFunction) (void* parameter, ClientReport report);
  * Otherwise the internal data structures storing the received data set values will not be updated
  * correctly.
  *
- * When replacing a report handler you only have to call this function. There is no separate call to
+ * \note Replacing a report handler you only have to call this function. There is no separate call to
  * IedConnection_uninstallReportHandler() required.
  *
+ * \note Do not call this function inside of the ReportCallbackFunction. Doing so will cause a deadlock.
+ *
  * \param self the connection object
  * \param rcbReference object reference of the report control block
  * \param rptId a string that identifies the report. If the rptId is not available then the
@@ -1273,6 +1275,8 @@ IedConnection_installReportHandler(IedConnection self, const char* rcbReference,
 /**
  * \brief uninstall a report handler function for the specified report control block (RCB)
  *
+ * \note Do not call this function inside of the ReportCallbackFunction. Doing so will cause a deadlock.
+ *
  * \param self the connection object
  * \param rcbReference object reference of the report control block
  */