refactor(core): Simplify UA_LOCK_ASSERT

jpfr · Sep 10, 2024 · 1654a28 · 1654a28
1 parent de58380
commit 1654a28
Show file tree

Hide file tree

Showing 36 changed files with 264 additions and 260 deletions.
diff --git a/include/open62541/config.h.in b/include/open62541/config.h.in
@@ -339,41 +339,44 @@ extern UA_THREAD_LOCAL void * (*UA_reallocSingleton)(void *ptr, size_t size);
 # define UA_LOCK_DESTROY(lock)
 # define UA_LOCK(lock)
 # define UA_UNLOCK(lock)
-# define UA_LOCK_ASSERT(lock, num)
+# define UA_LOCK_ASSERT(lock)
 
 #elif defined(UA_ARCHITECTURE_WIN32)
 
 typedef struct {
     CRITICAL_SECTION mutex;
-    int mutexCounter;
+    bool flag; /* For assertions that we hold the mutex */
 } UA_Lock;
 
 static UA_INLINE void
 UA_LOCK_INIT(UA_Lock *lock) {
     InitializeCriticalSection(&lock->mutex);
-    lock->mutexCounter = 0;
+    lock->flag = false;
 }
 
 static UA_INLINE void
 UA_LOCK_DESTROY(UA_Lock *lock) {
+    UA_assert(!lock->flag);
     DeleteCriticalSection(&lock->mutex);
 }
 
 static UA_INLINE void
 UA_LOCK(UA_Lock *lock) {
     EnterCriticalSection(&lock->mutex);
-    UA_assert(++(lock->mutexCounter) == 1);
+    UA_assert(!lock->flag);
+    lock->flag = true;
 }
 
 static UA_INLINE void
 UA_UNLOCK(UA_Lock *lock) {
-    UA_assert(--(lock->mutexCounter) == 0);
+    UA_assert(lock->flag);
+    lock->flag = false;
     LeaveCriticalSection(&lock->mutex);
 }
 
 static UA_INLINE void
-UA_LOCK_ASSERT(UA_Lock *lock, int num) {
-    UA_assert(lock->mutexCounter == num);
+UA_LOCK_ASSERT(UA_Lock *lock) {
+    UA_assert(lock->flag);
 }
 
 #elif defined(UA_ARCHITECTURE_POSIX)
@@ -382,39 +385,40 @@ UA_LOCK_ASSERT(UA_Lock *lock, int num) {
 
 typedef struct {
     pthread_mutex_t mutex;
-    int mutexCounter;
+    bool flag; /* For assertions that we hold the mutex */
 } UA_Lock;
 
-#define UA_LOCK_STATIC_INIT {PTHREAD_MUTEX_INITIALIZER, 0}
+#define UA_LOCK_STATIC_INIT {PTHREAD_MUTEX_INITIALIZER, false}
 
 static UA_INLINE void
 UA_LOCK_INIT(UA_Lock *lock) {
     pthread_mutex_init(&lock->mutex, NULL);
-    lock->mutexCounter = 0;
+    lock->flag = false;
 }
 
 static UA_INLINE void
 UA_LOCK_DESTROY(UA_Lock *lock) {
+    UA_assert(!lock->flag);
     pthread_mutex_destroy(&lock->mutex);
 }
 
 static UA_INLINE void
 UA_LOCK(UA_Lock *lock) {
     pthread_mutex_lock(&lock->mutex);
-    UA_assert(lock->mutexCounter == 0);
-    lock->mutexCounter++;
+    UA_assert(!lock->flag);
+    lock->flag = true;
 }
 
 static UA_INLINE void
 UA_UNLOCK(UA_Lock *lock) {
-    UA_assert(lock->mutexCounter == 1);
-    lock->mutexCounter--;
+    UA_assert(lock->flag);
+    lock->flag = false;
     pthread_mutex_unlock(&lock->mutex);
 }
 
 static UA_INLINE void
-UA_LOCK_ASSERT(UA_Lock *lock, int num) {
-    UA_assert(lock->mutexCounter == num);
+UA_LOCK_ASSERT(UA_Lock *lock) {
+    UA_assert(lock->flag);
 }
 
 #endif

diff --git a/src/client/ua_client.c b/src/client/ua_client.c
@@ -272,7 +272,7 @@ static const char *sessionStateTexts[6] =
 
 void
 notifyClientState(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     if(client->connectStatus == client->oldConnectStatus &&
        client->channel.state == client->oldChannelState &&
@@ -322,7 +322,7 @@ notifyClientState(UA_Client *client) {
 static UA_StatusCode
 sendRequest(UA_Client *client, const void *request,
             const UA_DataType *requestType, UA_UInt32 *requestId) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     /* Renew SecureChannel if necessary */
     __Client_renewSecureChannel(client);
@@ -751,7 +751,7 @@ __Client_AsyncService(UA_Client *client, const void *request,
                       UA_ClientAsyncServiceCallback callback,
                       const UA_DataType *responseType,
                       void *userdata, UA_UInt32 *requestId) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     /* Is the SecureChannel connected? */
     if(client->channel.state != UA_SECURECHANNELSTATE_OPEN) {
@@ -1022,7 +1022,7 @@ clientHouseKeeping(UA_Client *client, void *_) {
 
 UA_StatusCode
 __UA_Client_startup(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     UA_EventLoop *el = client->config.eventLoop;
     UA_CHECK_ERROR(el != NULL,

diff --git a/src/client/ua_client_connect.c b/src/client/ua_client_connect.c
@@ -667,7 +667,7 @@ sendOPNAsync(UA_Client *client, UA_Boolean renew) {
 
 UA_StatusCode
 __Client_renewSecureChannel(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     UA_EventLoop *el = client->config.eventLoop;
     UA_DateTime now = el->dateTime_nowMonotonic(el);
@@ -753,7 +753,7 @@ responseActivateSession(UA_Client *client, void *userdata,
 
 static UA_StatusCode
 activateSessionAsync(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     if(client->sessionState != UA_SESSIONSTATE_CREATED &&
        client->sessionState != UA_SESSIONSTATE_ACTIVATED) {
@@ -1087,7 +1087,7 @@ responseGetEndpoints(UA_Client *client, void *userdata,
 
 static UA_StatusCode
 requestGetEndpoints(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     UA_GetEndpointsRequest request;
     UA_GetEndpointsRequest_init(&request);
@@ -1271,7 +1271,7 @@ createSessionCallback(UA_Client *client, void *userdata,
 
 static UA_StatusCode
 createSessionAsync(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     /* Generate the local nonce for the session */
     UA_StatusCode res = UA_STATUSCODE_GOOD;
@@ -1352,7 +1352,7 @@ initSecurityPolicy(UA_Client *client) {
 
 static void
 connectActivity(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
     UA_LOG_TRACE(client->config.logging, UA_LOGCATEGORY_CLIENT,
                  "Client connect iterate");
 
@@ -1639,7 +1639,7 @@ __Client_networkCallback(UA_ConnectionManager *cm, uintptr_t connectionId,
 /* Initialize a TCP connection. Writes the result to client->connectStatus. */
 static void
 initConnect(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
     if(client->channel.state != UA_SECURECHANNELSTATE_CLOSED) {
         UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_CLIENT,
                        "Client connection already initiated");
@@ -1727,7 +1727,7 @@ initConnect(UA_Client *client) {
 
 void
 connectSync(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     /* EventLoop is started. Otherwise initConnect would have failed. */
     UA_EventLoop *el = client->config.eventLoop;
@@ -1774,7 +1774,7 @@ connectSync(UA_Client *client) {
 
 UA_StatusCode
 connectInternal(UA_Client *client, UA_Boolean async) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     /* Reset the connectStatus. This should be the only place where we can
      * recover from a bad connectStatus. */
@@ -1790,7 +1790,7 @@ connectInternal(UA_Client *client, UA_Boolean async) {
 
 UA_StatusCode
 connectSecureChannel(UA_Client *client, const char *endpointUrl) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     UA_ClientConfig *cc = UA_Client_getConfig(client);
     cc->noSession = true;
@@ -1809,7 +1809,7 @@ __UA_Client_connect(UA_Client *client, UA_Boolean async) {
 
 static UA_StatusCode
 activateSessionSync(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     /* EventLoop is started. Otherwise activateSessionAsync would have failed. */
     UA_EventLoop *el = client->config.eventLoop;

diff --git a/src/client/ua_client_discovery.c b/src/client/ua_client_discovery.c
@@ -40,7 +40,7 @@ static UA_StatusCode
 getEndpointsInternal(UA_Client *client, const UA_String endpointUrl,
                      size_t *endpointDescriptionsSize,
                      UA_EndpointDescription **endpointDescriptions) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     UA_GetEndpointsRequest request;
     UA_GetEndpointsRequest_init(&request);

diff --git a/src/client/ua_client_subscriptions.c b/src/client/ua_client_subscriptions.c
@@ -53,7 +53,7 @@ MonitoredItem_delete(UA_Client *client, UA_Client_Subscription *sub,
 static void
 ua_Subscriptions_create(UA_Client *client, UA_Client_Subscription *newSub,
                         UA_CreateSubscriptionResponse *response) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     UA_EventLoop *el = client->config.eventLoop;
 
@@ -425,7 +425,7 @@ UA_Client_Subscriptions_deleteSingle(UA_Client *client, UA_UInt32 subscriptionId
 static void
 MonitoredItem_delete(UA_Client *client, UA_Client_Subscription *sub,
                      UA_Client_MonitoredItem *mon) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     ZIP_REMOVE(MonitorItemsTree, &sub->monitoredItems, mon);
     if(mon->deleteCallback) {
@@ -642,7 +642,7 @@ createDataChanges_async(UA_Client *client, const UA_CreateMonitoredItemsRequest
                         UA_Client_DeleteMonitoredItemCallback *deleteCallbacks,
                         UA_ClientAsyncServiceCallback createCallback, void *userdata,
                         UA_UInt32 *requestId) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     UA_Client_Subscription *sub = findSubscription(client, request.subscriptionId);
     if(!sub)
@@ -1014,7 +1014,7 @@ UA_Client_MonitoredItems_modify_async(UA_Client *client,
 /* Assume the request is already initialized */
 UA_StatusCode
 __Client_preparePublishRequest(UA_Client *client, UA_PublishRequest *request) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     /* Count acks */
     UA_Client_NotificationsAckNumber *ack;
@@ -1055,7 +1055,7 @@ __nextSequenceNumber(UA_UInt32 sequenceNumber) {
 static void
 processDataChangeNotification(UA_Client *client, UA_Client_Subscription *sub,
                               UA_DataChangeNotification *dataChangeNotification) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     for(size_t j = 0; j < dataChangeNotification->monitoredItemsSize; ++j) {
         UA_MonitoredItemNotification *min = &dataChangeNotification->monitoredItems[j];
@@ -1095,7 +1095,7 @@ processDataChangeNotification(UA_Client *client, UA_Client_Subscription *sub,
 static void
 processEventNotification(UA_Client *client, UA_Client_Subscription *sub,
                          UA_EventNotificationList *eventNotificationList) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     for(size_t j = 0; j < eventNotificationList->eventsSize; ++j) {
         UA_EventFieldList *eventFieldList = &eventNotificationList->events[j];
@@ -1136,7 +1136,7 @@ processEventNotification(UA_Client *client, UA_Client_Subscription *sub,
 static void
 processNotificationMessage(UA_Client *client, UA_Client_Subscription *sub,
                            UA_ExtensionObject *msg) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     if(msg->encoding != UA_EXTENSIONOBJECT_DECODED)
         return;
@@ -1181,7 +1181,7 @@ processNotificationMessage(UA_Client *client, UA_Client_Subscription *sub,
 static void
 __Client_Subscriptions_processPublishResponse(UA_Client *client, UA_PublishRequest *request,
                                               UA_PublishResponse *response) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     UA_NotificationMessage *msg = &response->notificationMessage;
 
@@ -1341,7 +1341,7 @@ __Client_Subscriptions_clear(UA_Client *client) {
 
 void
 __Client_Subscriptions_backgroundPublishInactivityCheck(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     if(client->sessionState < UA_SESSIONSTATE_ACTIVATED)
         return;
@@ -1377,7 +1377,7 @@ __Client_Subscriptions_backgroundPublishInactivityCheck(UA_Client *client) {
 
 void
 __Client_Subscriptions_backgroundPublish(UA_Client *client) {
-    UA_LOCK_ASSERT(&client->clientMutex, 1);
+    UA_LOCK_ASSERT(&client->clientMutex);
 
     if(client->sessionState != UA_SESSIONSTATE_ACTIVATED)
         return;