Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NodeState.RemoveReference may remove the wrong reference #2446

Closed
1 of 5 tasks
einarmo opened this issue Jan 3, 2024 · 5 comments · Fixed by #2450
Closed
1 of 5 tasks

NodeState.RemoveReference may remove the wrong reference #2446

einarmo opened this issue Jan 3, 2024 · 5 comments · Fixed by #2450
Assignees
Labels
bug A bug was identified and should be fixed.

Comments

@einarmo
Copy link
Contributor

einarmo commented Jan 3, 2024

Type of issue

  • Bug
  • Enhancement
  • Compliance
  • Question
  • Help wanted

Current Behavior

NodeState.RemoveReference now removes the first reference that happens to have the requested TargetId, which will often be incorrect. This was changed in #2345, but the fix does not make sense to me.

Expected Behavior

RemoveReference is a public method that is stated to remove the reference defined by (referenceTypeId, isInverse, targetId), which it currently does not do. It also incorrectly reports which reference it did remove.

Steps To Reproduce

myState.AddReference(ReferenceTypeIds.Organizes, false, nodeId);
myState.AddReference(ReferenceTypeIds.GeneratesEvent, false, nodeId);
myState.RemoveReference(ReferenceTypeIds.GeneratesEvent, false, nodeId);
// Throws an exception, due to a conflict.
myState.AddReference(ReferenceTypeIds.GeneratesEvent, false, nodeId);

Environment

- OS: Linux, Fedora
- Environment: CLI
- Runtime: .NET 8
- Nuget Version: 6.8.0.32767
- Component: Opc.Ua.Core
- Server: My own server, but this should be reproducible in the reference server
- Client: None

Anything else?

No response

@mregen mregen added the bug A bug was identified and should be fixed. label Jan 3, 2024
@mregen
Copy link
Contributor

mregen commented Jan 4, 2024

Hi @einarmo, please check the modified fix and let me know if it is back working.

@mregen mregen added this to the 1.4.372 Januar update preview milestone Jan 4, 2024
@einarmo
Copy link
Contributor Author

einarmo commented Jan 4, 2024

This fixes my issue, but it still seems like weird behavior for RemoveReference to do this. It is very unintuitive that it can in some cases remove a different reference from the one you asked to remove, but I don't quite know what #2343 actually is.

@mregen
Copy link
Contributor

mregen commented Jan 4, 2024

I think your opinion is correct. I double checked and it looks like the error happens in another layer. Once verified, we go back to the original code in NodeState. Thanks for checking!

@mregen
Copy link
Contributor

mregen commented Jan 12, 2024

The regression qualifies for a hotfix for .106

@mregen
Copy link
Contributor

mregen commented Jan 16, 2024

see 1.4.372.107

@mregen mregen closed this as completed Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug was identified and should be fixed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants