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

StreamManager.Close() -> segfault #107

Closed
Kroev opened this issue Sep 5, 2019 · 2 comments
Closed

StreamManager.Close() -> segfault #107

Kroev opened this issue Sep 5, 2019 · 2 comments
Assignees
Labels

Comments

@Kroev
Copy link

Kroev commented Sep 5, 2019

Hello,

There is an issue with calling StreamManager.Close() causing a segfault in certain situations:

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

goroutine 1 [running]:
gosrc.io/xmpp.(*Client).Disconnect(0xc4200a9860)
        /go/src/gosrc.io/xmpp/client.go:192 +0x50
gosrc.io/xmpp.(*StreamManager).Stop(0xc42014c630)
        /go/src/gosrc.io/xmpp/stream_manager.go:106 +0x52
[...]

As far as I can tell this happens

  1. if the client is not yet connected
  2. if the connection went down (lost tcp connection and then calling SM.Close() )

I underestand that these are situations where closing the SM should not be necessary but there is no way to tell.

I am thinking of two different approaches to fix this:

  • make client object accessable from the sm, so something like sm.Client.CurrentState is possible
  • as I understand why you would not want to let people fiddle with the client in a SM, a getter for the client state: sm.CurrentState() or similar

This would enable me to tell wehther I need to close the SM or not.

Kroev

@mremond
Copy link
Member

mremond commented Sep 5, 2019

I think a better approach would be to guard against the crash and let the client call Close at any time, no matter what the state is.

@mremond mremond added the bug label Sep 5, 2019
@mremond mremond self-assigned this Sep 6, 2019
@mremond
Copy link
Member

mremond commented Sep 6, 2019

It should be fixed by this commit: 8794ea6

Could you please reopen the issue if this is not the case?

Thanks for the report!

@mremond mremond closed this as completed Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants