Skip to content
This repository has been archived by the owner on Dec 14, 2020. It is now read-only.

nil pointer dereference #33

Closed
amenzhinsky opened this issue Feb 14, 2018 · 4 comments · Fixed by #34
Closed

nil pointer dereference #33

amenzhinsky opened this issue Feb 14, 2018 · 4 comments · Fixed by #34

Comments

@amenzhinsky
Copy link

amenzhinsky commented Feb 14, 2018

Randomly in my CI pipeline I get panics like:

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

goroutine 189 [running]:
.../vendor/pack.ag/amqp.(*Message).shouldSendDisposition(...)
	vendor/pack.ag/amqp/types.go:1741
.../vendor/pack.ag/amqp.(*Message).Accept(0x0)
	.../olu-advance-connector/vendor/pack.ag/amqp/types.go:1720 +0x22

Revision: 156a96cbd71de6a80dd774d60de212f31c7272c7
If it's needed I can spend some time trying to narrow down the issue or you already have thoughts on this?

P.S. updated to the latest version, we'll see how it goes.

@vcabbage
Copy link
Owner

vcabbage commented Feb 14, 2018

pack.ag/amqp.(*Message).Accept(0x0)

This shows that Accept() is being called on a nil Message. Receiver.Receive() should only return a nil Message in the case of an error.

Can you verify that you're handling the error from Receiver.Receive() and not letting the code continue on and call msg.Accept()? In tests it's pretty easy to get this sort of issue when using t.Error() instead of t.Fatal().

@vcabbage
Copy link
Owner

vcabbage commented Feb 14, 2018

On closer inspection, it looks like there is a case where Receiver.Receive() can return a nil Message and nil error. I'll prepare a fix for this case.

Thank you for bringing this to me attention.

amqp/client.go

Lines 1231 to 1232 in a3e580d

case <-r.link.done:
return nil, r.link.err

@amenzhinsky
Copy link
Author

Seems like this is it, because I'm closing a link in a separate goroutine.

vcabbage added a commit that referenced this issue Feb 14, 2018
… error.

* Adds exported `ErrConnClosed`, `ErrSessionClosed`, and `ErrLinkClosed`
  indicating a library consumer requested close.
* Some refactoring to conn close logic to remove race condition where
  another component could acquire conn.errMu after conn.mux releases it
  but before conn.close acquires it.
* Copied integration test fixes from #31.

Fixes #33
vcabbage added a commit that referenced this issue Feb 14, 2018
… error. (#34)

* Adds exported `ErrConnClosed`, `ErrSessionClosed`, and `ErrLinkClosed`
  indicating a library consumer requested close.
* Some refactoring to conn close logic to remove race condition where
  another component could acquire conn.errMu after conn.mux releases it
  but before conn.close acquires it.
* Copied integration test fixes from #31.

Fixes #33
@vcabbage
Copy link
Owner

Fix is in master, let me know if that doesn't fix your issue or if you run into any other problems.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants