Open aykevl opened 6 days ago
Also, I think ap.connMu should have been used inside pongAckTimer?
I have mixed feelings about this: yes it should be held for reading, but at the same time if we end up waiting because it is held for writing and then call close on it we are probably closing a connection that has just been created. Citing my own comment:
Main problem should be fixed with 095020537ece27f8967a14819edfa4ed10350145, same issue as 0b32df773c9cbe12a2bc44e92580d0c657a170a2.
I have mixed feelings about this: yes it should be held for reading, but at the same time if we end up waiting because it is held for writing and then call close on it we are probably closing a connection that has just been created.
It's still a data race, which is basically undefined behavior (though it might work well enough in practice since it's a pointer read, which will normally not be torn).
Maybe I'll take a stab at fixing this at some point.
I got this crash, I think when the network connection got disconnected:
And this crash, probably the same bug, when the network definitely got disconnected (I think there was some maintenance on the router).
I suspect the issue is here:
https://github.com/devgianlu/go-librespot/blob/f2de517abcf49bb58eb0ccfed6319bd58df94d81/ap/ap.go#L82
This is
ap.init
, which is called fromap.connect
, which is called fromap.reconnect
. Looks like it setsap.conn
to nil when it can't connect resulting in the nil pointer dereference.(Also, I think
ap.connMu
should have been used insidepongAckTimer
?)I should also say it's quite impressive that the daemon had been running crash-free for over 10 days!