Deadlock at disconnect

[troian]

Pretty often I see client hangs at disconnect stage
Here is log

[net] done putting msg on incomingPubChan
[net] putting puback msg on obound
[net] obound priority msg to write, type *packets.DisconnectPacket
[net] outbound wrote disconnect, stopping
[pinger] keepalive stopped
[net] incoming stopped

You might notice that there is no message done putting puback msg on obound because of the channel oboundP is full and outgoing worker has finished it's job before alllogic.
Quick workaround is to increase channel depth by SetMessageChannelDepth but it's not a case when there is extensive message exchange

[troian]

At first sight it seems worth to remove return at net.go:203 which gives alllogic chance to finish properly without deadlock. But might break some internal usecase

[ghost]

Could you have a look at #122. I think it's the same issue.

Another alternative could be to have a cascading teardown of goroutines when a stop is received by all 4 goroutines being waited on (keepalive optional).

[troian]

@pshirali #122 looks same. +1 one for cascading shutdown

[troian]

Close as dup of #122

[troian]

Reopen as seems deadlock still persists on recent codebase
See attached goroutines dump goroutines.txt

[ghost]

I have the same problem: hang on with disconnect() method of client.go, at c.workers.Wait().
I use iot.eclipse.org broker, and send 1 message per second. It hangs randomly between minutes and hours.
I have bypassed the problem for now by using a single connection for publications.

[troian]

@sdariz if possible for your implementation you can temporary set keep alive to 0 as a workaround

[ghost]

@troian Thanks, I'm going to try

[alsm]

Thanks for reporting it again, I think I was getting far to complicated with the keepalive and timers, just trying something much simpler than should be better.

[alsm]

Please test the above, if it's good I'll do a point release on Monday

[troian]

looks good for me. At least no hangs at my tests

[ghost]

Same as troian. No hangs until now. Seems ok

[ghost]

I currently haven't integrated your latest changes e020008, as there was a national holiday here in India. I'm going to integrate your changes today and report by tomorrow or day-after.

I had previously integrated aff1577. I did find some hangs on a long running instance (in the goroutines spawned inside keepalive), but that code doesn't exist in e020008. So, hopefully, there may be no hangs now.

[alsm]

Any feedback on this @pshirali ?

[ghost]

I've run 50 instances of my program for 6 days now, with a disconnect-reconnect happening every 40 seconds. No issues!. Great work @alsm

You might also want to checkout a comment at the bottom of this commit though.

[daenney]

I just upgraded to master and I get similar results as @pshirali. We had some behaviour that very much seemed like a deadlock, it always happened after mosquitto would log a socket error and see the client disconnect, but never come back. Ran with master instead of v1.1.0 for the past 24hrs and so far none of it. Interestingly enough the "socket error"s on mosquitto's side seem to also have disappeared, whereas before we'd have about 10 of those over the course of 24hrs.

[alsm]

I'm going to call this done then, thanks to everyone for the details and feedback you provided.