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.

Sign up for GitHub

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

Jump to bottom

Panic in secure_channel.go #640

Closed
aschiffmann opened this issue Mar 3, 2023 · 2 comments · Fixed by #654
Closed

Panic in secure_channel.go #640

aschiffmann opened this issue Mar 3, 2023 · 2 comments · Fixed by #654
Assignees
@magiconair
Labels
bug Something isn't working
Milestone
v0.3.15

Comments

@aschiffmann
Copy link

Hello,
I got a panic when calling ReadWithContext().
I cannot reproduce the error, I was calling the function a huge number of times before and without any change in the setup I randomly got this panic once.

Do you have an idea what could be the reason for it and how to prevent it?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x68 pc=0x7eafef]

goroutine 63 [running]:
github.com/gopcua/opcua/uasc.(*SecureChannel).SendRequestWithTimeoutWithContext(0x0, {0xc63d20, 0xc00003c058}, {0xc618d8, 0xc000410360}, 0xa053c0?, 0xc0004dda80?, 0x10bd360?)
github.com/gopcua/opcua@v0.3.11/uasc/secure_channel.go:748 +0x4f
github.com/gopcua/opcua.(*Client).sendWithTimeout(0x108c8a0?, {0xc63d20?, 0xc00003c058?}, {0xc618d8?, 0xc000410360?}, 0xc0004ddaf0?, 0x40f9a7?)
github.com/gopcua/opcua@v0.3.11/client.go:942 +0x136
github.com/gopcua/opcua.(*Client).SendWithContext(0xc00058c690, {0xc63d20, 0xc00003c058}, {0xc618d8, 0xc000410360}, 0xa9a420?)
github.com/gopcua/opcua@v0.3.11/client.go:925 +0x91
github.com/gopcua/opcua.(*Client).ReadWithContext(0xa320c0?, {0xc63d20, 0xc00003c058}, 0xc0003e3b00)
github.com/gopcua/opcua@v0.3.11/client.go:1014 +0x29c
@magiconair
Copy link
Member

magiconair commented Mar 6, 2023 •
edited
Loading

I think this is caused by this line which re-creates the secure channel and sets the internal secure channel instance temporarily to nil.

https://github.com/gopcua/opcua/blob/v0.3.11/client.go#L355

We don't catch this properly so this line fails with a nil pointer error since c.SecureChannel() returns nil until the channel has been restored.

https://github.com/gopcua/opcua/blob/v0.3.11/client.go#L942

Need to think about this a bit. What response should we return?

@magiconair magiconair added the bug Something isn't working label Mar 6, 2023
@magiconair magiconair self-assigned this May 24, 2023
@magiconair magiconair added this to the v0.3.15 milestone May 24, 2023
@magiconair
Copy link
Member

Looking at the code I am finding some race conditions like this one. The initial nil check does not trigger but when we make the actual send request it fails since the connection is re-established.

image

magiconair added a commit that referenced this issue May 25, 2023
@magiconair
Fix races to c.Session() and c.SecureChannel()
5a15559 
In these cases the method calls c.SecureChannel and/or c.Session multiple
times and during a re-connect the returned value can change. This patch
addresses this by storing the returned value in a local variable.

Fixes #640
@magiconair magiconair mentioned this issue May 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@magiconair magiconair

Labels
bug Something isn't working
Projects
None yet
Milestone
v0.3.15
Development

Successfully merging a pull request may close this issue.

Fix races to c.Session() and c.SecureChannel() gopcua/opcua
2 participants
@magiconair @aschiffmann