From cdd0684ffbd77ccf86bcae5d69fd8ceac13e124a Mon Sep 17 00:00:00 2001
From: Michael Zillgith <michael.zillgith@mz-automation.de>
Date: Thu, 15 Feb 2024 20:12:07 +0000
Subject: [PATCH] - fixed - potential race condition when using
 IedConnection_installReportHandler and IedConnection_uninstallReportHandler

---
 src/iec61850/client/client_report.c | 33 ++++++++++++++++++-----------
 src/iec61850/inc/iec61850_client.h  |  8 +++++--
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/src/iec61850/client/client_report.c b/src/iec61850/client/client_report.c
index fbaa74c2..888968dd 100644
--- 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
index c3a174fe..8a6f5371 100644
--- 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
  */