Skip to content

Commit

Permalink
fix(server): Don't assert that a lock was not taken -- a concurrent t…
Browse files Browse the repository at this point in the history
…hread might take it
  • Loading branch information
jpfr committed May 23, 2024
1 parent ab1e309 commit 88c92c9
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 60 deletions.
2 changes: 0 additions & 2 deletions src/client/ua_client_subscriptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ ua_Subscriptions_create(UA_Client *client, UA_Client_Subscription *newSub,
static void
ua_Subscriptions_create_handler(UA_Client *client, void *data,
UA_UInt32 requestId, void *r) {
UA_LOCK_ASSERT(&client->clientMutex, 0);

UA_CreateSubscriptionResponse *response = (UA_CreateSubscriptionResponse *)r;
CustomCallback *cc = (CustomCallback *)data;
UA_Client_Subscription *newSub = (UA_Client_Subscription *)cc->clientData;
Expand Down
12 changes: 0 additions & 12 deletions src/pubsub/ua_pubsub_ns0.c
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,6 @@ addPubSubConnectionAction(UA_Server *server,
const UA_NodeId *objectId, void *objectContext,
size_t inputSize, const UA_Variant *input,
size_t outputSize, UA_Variant *output) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_StatusCode res = addPubSubConnectionLocked(server, sessionId, sessionHandle,
methodId, methodContext,
Expand All @@ -824,7 +823,6 @@ removeConnectionAction(UA_Server *server,
const UA_NodeId *objectId, void *objectContext,
size_t inputSize, const UA_Variant *input,
size_t outputSize, UA_Variant *output){
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_StatusCode retVal = UA_STATUSCODE_GOOD;
UA_NodeId nodeToRemove = *((UA_NodeId *) input[0].data);
retVal |= UA_Server_removePubSubConnection(server, nodeToRemove);
Expand Down Expand Up @@ -967,8 +965,6 @@ addDataSetFolderAction(UA_Server *server,
const UA_NodeId *objectId, void *objectContext,
size_t inputSize, const UA_Variant *input,
size_t outputSize, UA_Variant *output){
UA_LOCK_ASSERT(&server->serviceMutex, 0);

/* defined in R 1.04 9.1.4.5.7 */
UA_StatusCode retVal = UA_STATUSCODE_GOOD;
UA_String newFolderName = *((UA_String *) input[0].data);
Expand Down Expand Up @@ -1102,7 +1098,6 @@ addPublishedDataItemsAction(UA_Server *server,
const UA_NodeId *objectId, void *objectContext,
size_t inputSize, const UA_Variant *input,
size_t outputSize, UA_Variant *output){
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_StatusCode retVal = UA_STATUSCODE_GOOD;
size_t fieldNameAliasesSize = input[1].arrayLength;
UA_String * fieldNameAliases = (UA_String *) input[1].data;
Expand Down Expand Up @@ -1278,8 +1273,6 @@ readContentMask(UA_Server *server, const UA_NodeId *sessionId,
void *sessionContext, const UA_NodeId *nodeId,
void *nodeContext, UA_Boolean includeSourceTimeStamp,
const UA_NumericRange *range, UA_DataValue *value) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_WriterGroup *writerGroup = (UA_WriterGroup*)nodeContext;
if((writerGroup->config.messageSettings.encoding != UA_EXTENSIONOBJECT_DECODED &&
writerGroup->config.messageSettings.encoding != UA_EXTENSIONOBJECT_DECODED_NODELETE) ||
Expand All @@ -1300,8 +1293,6 @@ writeContentMask(UA_Server *server, const UA_NodeId *sessionId,
void *sessionContext, const UA_NodeId *nodeId,
void *nodeContext, const UA_NumericRange *range,
const UA_DataValue *value) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_WriterGroup *writerGroup = (UA_WriterGroup*)nodeContext;
if((writerGroup->config.messageSettings.encoding != UA_EXTENSIONOBJECT_DECODED &&
writerGroup->config.messageSettings.encoding != UA_EXTENSIONOBJECT_DECODED_NODELETE) ||
Expand Down Expand Up @@ -1465,8 +1456,6 @@ removeGroupAction(UA_Server *server,
const UA_NodeId *objectId, void *objectContext,
size_t inputSize, const UA_Variant *input,
size_t outputSize, UA_Variant *output){
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_NodeId nodeToRemove = *((UA_NodeId *)input->data);
if(UA_WriterGroup_findWGbyId(server, nodeToRemove)) {
UA_WriterGroup *wg = UA_WriterGroup_findWGbyId(server, nodeToRemove);
Expand Down Expand Up @@ -1858,7 +1847,6 @@ removeDataSetWriterAction(UA_Server *server,
const UA_NodeId *objectId, void *objectContext,
size_t inputSize, const UA_Variant *input,
size_t outputSize, UA_Variant *output){
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_NodeId nodeToRemove = *((UA_NodeId *) input[0].data);
return UA_Server_removeDataSetWriter(server, nodeToRemove);
}
Expand Down
1 change: 0 additions & 1 deletion src/server/ua_server_async.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ static UA_UInt32
processAsyncResults(UA_Server *server) {
UA_AsyncManager *am = &server->asyncManager;
UA_LOCK_ASSERT(&server->serviceMutex, 1);
UA_LOCK_ASSERT(&am->queueLock, 0);

UA_UInt32 count = 0;
UA_AsyncOperation *ao;
Expand Down
4 changes: 0 additions & 4 deletions src/server/ua_server_ns0_diagnostics.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ readSubscriptionDiagnostics(UA_Server *server,
const UA_NodeId *nodeId, void *nodeContext,
UA_Boolean sourceTimestamp,
const UA_NumericRange *range, UA_DataValue *value) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

/* Check the Subscription pointer */
UA_Subscription *sub = (UA_Subscription*)nodeContext;
if(!sub)
Expand Down Expand Up @@ -310,8 +308,6 @@ readSessionDiagnosticsArray(UA_Server *server,
const UA_NodeId *nodeId, void *nodeContext,
UA_Boolean sourceTimestamp,
const UA_NumericRange *range, UA_DataValue *value) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

/* Allocate the output array */
UA_SessionDiagnosticsDataType *sd = (UA_SessionDiagnosticsDataType*)
UA_Array_new(server->sessionCount,
Expand Down
41 changes: 0 additions & 41 deletions src/server/ua_subscription_alarms_conditions.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ UA_Server_setConditionTwoStateVariableCallback(UA_Server *server, const UA_NodeI
const UA_NodeId conditionSource, UA_Boolean removeBranch,
UA_TwoStateVariableChangeCallback callback,
UA_TwoStateVariableCallbackType callbackType) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);

/* Get Condition */
Expand Down Expand Up @@ -500,7 +499,6 @@ UA_Server_getNodeIdValueOfConditionField(UA_Server *server, const UA_NodeId *con
static UA_StatusCode
UA_Server_getConditionBranchNodeId(UA_Server *server, const UA_ByteString *eventId,
UA_NodeId *outConditionBranchNodeId) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);

*outConditionBranchNodeId = UA_NODEID_NULL;
Expand Down Expand Up @@ -541,7 +539,6 @@ static UA_StatusCode
UA_Server_getConditionLastSeverity(UA_Server *server, const UA_NodeId *conditionSource,
const UA_NodeId *conditionId, UA_UInt16 *outLastSeverity,
UA_DateTime *outLastSeveritySourceTimeStamp) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_Condition *cond = getCondition(server, conditionSource, conditionId);
if(!cond) {
Expand All @@ -560,7 +557,6 @@ static UA_StatusCode
UA_Server_updateConditionLastSeverity(UA_Server *server, const UA_NodeId *conditionSource,
const UA_NodeId *conditionId, UA_UInt16 lastSeverity,
UA_DateTime lastSeveritySourceTimeStamp) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_Condition *cond = getCondition(server, conditionSource, conditionId);
if(!cond) {
Expand All @@ -579,7 +575,6 @@ static UA_StatusCode
UA_Server_getConditionActiveState(UA_Server *server, const UA_NodeId *conditionSource,
const UA_NodeId *conditionId, UA_ActiveState *outLastActiveState,
UA_ActiveState *outCurrentActiveState, UA_Boolean *outIsLimitAlarm) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_Condition *cond = getCondition(server, conditionSource, conditionId);
if(!cond) {
Expand All @@ -599,7 +594,6 @@ static UA_StatusCode
UA_Server_updateConditionActiveState(UA_Server *server, const UA_NodeId *conditionSource,
const UA_NodeId *conditionId, const UA_ActiveState lastActiveState,
const UA_ActiveState currentActiveState, UA_Boolean isLimitAlarm) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_Condition *cond = getCondition(server, conditionSource, conditionId);
if(!cond) {
Expand Down Expand Up @@ -905,8 +899,6 @@ afterWriteCallbackEnabledStateChange(UA_Server *server,
const UA_NodeId *sessionId, void *sessionContext,
const UA_NodeId *nodeId, void *nodeContext,
const UA_NumericRange *range, const UA_DataValue *data) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

/* Callback for change in EnabledState/Id property.
* First we get the EnabledState NodeId then The Condition NodeId */
UA_NodeId twoStateVariableNode;
Expand Down Expand Up @@ -955,8 +947,6 @@ afterWriteCallbackAckedStateChange(UA_Server *server,
const UA_NodeId *sessionId, void *sessionContext,
const UA_NodeId *nodeId, void *nodeContext,
const UA_NumericRange *range, const UA_DataValue *data) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

/* Get the AckedState NodeId then The Condition NodeId */
UA_NodeId twoStateVariableNode;
UA_StatusCode retval = UA_Server_getFieldParentNodeId(server, nodeId, &twoStateVariableNode);
Expand Down Expand Up @@ -1050,8 +1040,6 @@ afterWriteCallbackConfirmedStateChange(UA_Server *server,
const UA_NodeId *sessionId, void *sessionContext,
const UA_NodeId *nodeId, void *nodeContext,
const UA_NumericRange *range, const UA_DataValue *data) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_Variant value;
UA_NodeId twoStateVariableNode;
UA_StatusCode retval = UA_Server_getFieldParentNodeId(server, nodeId, &twoStateVariableNode);
Expand Down Expand Up @@ -1145,8 +1133,6 @@ afterWriteCallbackActiveStateChange(UA_Server *server,
const UA_NodeId *sessionId, void *sessionContext,
const UA_NodeId *nodeId, void *nodeContext,
const UA_NumericRange *range, const UA_DataValue *data) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_Variant value;
UA_NodeId twoStateVariableNode;
UA_StatusCode retval = UA_Server_getFieldParentNodeId(server, nodeId, &twoStateVariableNode);
Expand Down Expand Up @@ -1292,8 +1278,6 @@ afterWriteCallbackSeverityChange(UA_Server *server,
const UA_NodeId *sessionId, void *sessionContext,
const UA_NodeId *nodeId, void *nodeContext,
const UA_NumericRange *range, const UA_DataValue *data) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_QualifiedName fieldLastSeverity = UA_QUALIFIEDNAME(0, CONDITION_FIELD_LASTSEVERITY);
UA_QualifiedName fieldSourceTimeStamp =
UA_QUALIFIEDNAME(0, CONDITION_FIELD_CONDITIONVARIABLE_SOURCETIMESTAMP);
Expand Down Expand Up @@ -1383,8 +1367,6 @@ disableMethodCallback(UA_Server *server, const UA_NodeId *sessionId,
void *methodContext, const UA_NodeId *objectId,
void *objectContext, size_t inputSize,
const UA_Variant *input, size_t outputSize, UA_Variant *output) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_NodeId conditionTypeNodeId = UA_NODEID_NUMERIC(0, UA_NS0ID_CONDITIONTYPE);
if(UA_NodeId_equal(objectId, &conditionTypeNodeId)) {
UA_LOG_WARNING(server->config.logging, UA_LOGCATEGORY_USERLAND,
Expand Down Expand Up @@ -1416,8 +1398,6 @@ enableMethodCallback(UA_Server *server, const UA_NodeId *sessionId,
void *objectContext, size_t inputSize,
const UA_Variant *input, size_t outputSize,
UA_Variant *output) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_NodeId conditionTypeNodeId = UA_NODEID_NUMERIC(0, UA_NS0ID_CONDITIONTYPE);
if(UA_NodeId_equal(objectId, &conditionTypeNodeId)) {
UA_LOG_WARNING(server->config.logging, UA_LOGCATEGORY_USERLAND,
Expand Down Expand Up @@ -1449,8 +1429,6 @@ addCommentMethodCallback(UA_Server *server, const UA_NodeId *sessionId,
void *objectContext, size_t inputSize,
const UA_Variant *input, size_t outputSize,
UA_Variant *output) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_QualifiedName fieldComment = UA_QUALIFIEDNAME(0, CONDITION_FIELD_COMMENT);
UA_QualifiedName fieldSourceTimeStamp =
UA_QUALIFIEDNAME(0, CONDITION_FIELD_CONDITIONVARIABLE_SOURCETIMESTAMP);
Expand Down Expand Up @@ -1537,8 +1515,6 @@ acknowledgeMethodCallback(UA_Server *server, const UA_NodeId *sessionId,
void *objectContext, size_t inputSize,
const UA_Variant *input, size_t outputSize,
UA_Variant *output) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_QualifiedName fieldComment = UA_QUALIFIEDNAME(0, CONDITION_FIELD_COMMENT);
UA_Variant value;

Expand Down Expand Up @@ -1619,8 +1595,6 @@ confirmMethodCallback(UA_Server *server, const UA_NodeId *sessionId,
void *objectContext, size_t inputSize,
const UA_Variant *input, size_t outputSize,
UA_Variant *output) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_QualifiedName fieldComment = UA_QUALIFIEDNAME(0, CONDITION_FIELD_COMMENT);
UA_Variant value;

Expand Down Expand Up @@ -1859,7 +1833,6 @@ refresh2MethodCallback(UA_Server *server, const UA_NodeId *sessionId,
void *objectContext, size_t inputSize,
const UA_Variant *input, size_t outputSize,
UA_Variant *output) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
//TODO implement logic for subscription array
/* Check if valid subscriptionId */
Expand Down Expand Up @@ -1903,7 +1876,6 @@ refreshMethodCallback(UA_Server *server, const UA_NodeId *sessionId,
void *objectContext, size_t inputSize,
const UA_Variant *input, size_t outputSize,
UA_Variant *output) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);

//TODO implement logic for subscription array
Expand Down Expand Up @@ -2555,8 +2527,6 @@ UA_Server_createCondition(UA_Server *server,
const UA_NodeId conditionSource,
const UA_NodeId hierarchialReferenceType,
UA_NodeId *outNodeId) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

if(!outNodeId) {
UA_LOG_ERROR(server->config.logging, UA_LOGCATEGORY_USERLAND,
"outNodeId cannot be NULL!");
Expand All @@ -2579,8 +2549,6 @@ UA_StatusCode
UA_Server_addCondition_begin(UA_Server *server, const UA_NodeId conditionId,
const UA_NodeId conditionType,
const UA_QualifiedName conditionName, UA_NodeId *outNodeId) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

if(!outNodeId) {
UA_LOG_ERROR(server->config.logging, UA_LOGCATEGORY_USERLAND,
"outNodeId cannot be NULL!");
Expand Down Expand Up @@ -2616,7 +2584,6 @@ UA_StatusCode
UA_Server_addCondition_finish(UA_Server *server, const UA_NodeId conditionId,
const UA_NodeId conditionSource,
const UA_NodeId hierarchialReferenceType) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);

const UA_Node *node = UA_NODESTORE_GET(server, &conditionId);
Expand Down Expand Up @@ -2806,7 +2773,6 @@ UA_StatusCode
UA_Server_addConditionOptionalField(UA_Server *server, const UA_NodeId condition,
const UA_NodeId conditionType, const UA_QualifiedName fieldName,
UA_NodeId *outOptionalNode) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_StatusCode res = addConditionOptionalField(server, condition, conditionType,
fieldName, outOptionalNode);
Expand Down Expand Up @@ -2840,7 +2806,6 @@ setConditionField(UA_Server *server, const UA_NodeId condition,
UA_StatusCode
UA_Server_setConditionField(UA_Server *server, const UA_NodeId condition,
const UA_Variant* value, const UA_QualifiedName fieldName) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_StatusCode retval = setConditionField(server, condition, value, fieldName);
UA_UNLOCK(&server->serviceMutex);
Expand Down Expand Up @@ -2888,7 +2853,6 @@ UA_Server_setConditionVariableFieldProperty(UA_Server *server, const UA_NodeId c
const UA_Variant* value,
const UA_QualifiedName variableFieldName,
const UA_QualifiedName variablePropertyName) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_StatusCode res = setConditionVariableFieldProperty(server, condition, value,
variableFieldName, variablePropertyName);
Expand Down Expand Up @@ -2937,7 +2901,6 @@ triggerConditionEvent(UA_Server *server, const UA_NodeId condition,
UA_StatusCode
UA_Server_triggerConditionEvent(UA_Server *server, const UA_NodeId condition,
const UA_NodeId conditionSource, UA_ByteString *outEventId) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);
UA_LOCK(&server->serviceMutex);
UA_StatusCode res = triggerConditionEvent(server, condition, conditionSource, outEventId);
UA_UNLOCK(&server->serviceMutex);
Expand All @@ -2947,8 +2910,6 @@ UA_Server_triggerConditionEvent(UA_Server *server, const UA_NodeId condition,
UA_StatusCode
UA_Server_deleteCondition(UA_Server *server, const UA_NodeId condition,
const UA_NodeId conditionSource) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

/* Get ConditionSource Entry */
UA_Boolean found = false; /* Delete from internal list */
UA_ConditionSource *source, *tmp_source;
Expand Down Expand Up @@ -3135,8 +3096,6 @@ UA_Server_setLimitState(UA_Server *server, const UA_NodeId conditionId,
UA_StatusCode
UA_Server_setExpirationDate(UA_Server *server, const UA_NodeId conditionId,
UA_ByteString cert) {
UA_LOCK_ASSERT(&server->serviceMutex, 0);

UA_StatusCode retval;
if(cert.data == NULL){
UA_LOG_ERROR(server->config.logging, UA_LOGCATEGORY_SERVER,
Expand Down

0 comments on commit 88c92c9

Please sign in to comment.