From philippe44/cspot PR #3 (open, 2026-08): "MercurySession: crash when a request races the reconnection (null conn/shanConn)". Our sessions reconnect every ~6 min, so the race is hit regularly. Drop this patch once the fix is in the pinned cspot commit. diff --git a/cspot/include/Session.h b/cspot/include/Session.h index df7f03c8..7cc8e32f 100644 --- a/cspot/include/Session.h +++ b/cspot/include/Session.h @@ -2,6 +2,7 @@ #include // for uint8_t #include // for shared_ptr, unique_ptr +#include // for mutex #include // for string #include // for vector @@ -23,6 +24,11 @@ class Session { std::shared_ptr conn; std::shared_ptr authBlob; + /* conn and shanConn are swapped by the session's task during reconnection, + * but other tasks send through them; they must hold this while copying the + * pointers, and the session's task while replacing them */ + std::mutex connMutex; + std::string deviceId = "142137fd329622137a14901634264e6f332e2411"; public: diff --git a/cspot/src/MercurySession.cpp b/cspot/src/MercurySession.cpp index 7aafbb76..29a6e546 100644 --- a/cspot/src/MercurySession.cpp +++ b/cspot/src/MercurySession.cpp @@ -66,8 +66,11 @@ void MercurySession::reconnect() { isReconnecting = true; try { - this->conn = nullptr; - this->shanConn = nullptr; + { + std::scoped_lock lock(connMutex); + this->conn = nullptr; + this->shanConn = nullptr; + } this->connectWithRandomAp(); this->authenticate(this->authBlob); @@ -127,7 +130,17 @@ void MercurySession::unregisterAudioKey(uint32_t sequenceId) { void MercurySession::disconnect() { CSPOT_LOG(info, "Disconnecting mercury session"); this->isRunning = false; - conn->close(); + + /* conn is null while a reconnection is in flight; isRunning above already + * makes the retry loop exit, closing is just to unblock a pending read */ + std::shared_ptr conn; + { + std::scoped_lock lock(connMutex); + conn = this->conn; + } + if (conn) + conn->close(); + std::scoped_lock lock(this->isRunningMutex); } @@ -305,8 +318,22 @@ uint64_t MercurySession::executeSubscription(RequestType method, // Bump sequence id this->sequenceId += 1; + /* the session's task may be swapping shanConn for a reconnection right now, + * and dereferencing it here would not be a catchable failure, so take a + * snapshot; when disconnected, the request is simply lost */ + std::shared_ptr shanConn; + { + std::scoped_lock lock(connMutex); + shanConn = this->shanConn; + } + + if (!shanConn) { + CSPOT_LOG(info, "Mercury request skipped, session is reconnecting"); + return this->sequenceId - 1; + } + try { - this->shanConn->sendPacket( + shanConn->sendPacket( static_cast::type>(method), sequenceIdBytes); } catch (...) { @@ -337,8 +364,21 @@ uint32_t MercurySession::requestAudioKey(const std::vector& trackId, // Used for broken connection detection // this->lastRequestTimestamp = timeProvider->getSyncedTimestamp(); + + // same snapshot as executeSubscription: this runs on the track queue's task + std::shared_ptr shanConn; + { + std::scoped_lock lock(connMutex); + shanConn = this->shanConn; + } + + if (!shanConn) { + CSPOT_LOG(info, "Audio key request skipped, session is reconnecting"); + return audioKeySequence - 1; + } + try { - this->shanConn->sendPacket( + shanConn->sendPacket( static_cast(RequestType::AUDIO_KEY_REQUEST_COMMAND), buffer); } catch (...) { // @TODO: Handle disconnect diff --git a/cspot/src/Session.cpp b/cspot/src/Session.cpp index 74bd0633..be154bbe 100644 --- a/cspot/src/Session.cpp +++ b/cspot/src/Session.cpp @@ -4,6 +4,7 @@ #include // for uint8_t #include // for __base #include // for shared_ptr, unique_ptr, make_unique +#include // for scoped_lock #include // for default_random_engine, independent_bi... #include // for remove_extent_t #include // for move @@ -33,7 +34,10 @@ Session::Session() { Session::~Session() {} void Session::connect(std::unique_ptr connection) { - this->conn = std::move(connection); + { + std::scoped_lock lock(connMutex); + this->conn = std::move(connection); + } conn->timeoutHandler = [this]() { return this->triggerTimeout(); }; @@ -48,11 +52,15 @@ void Session::connect(std::unique_ptr connection) { CSPOT_LOG(debug, "Received shannon keys"); // Generates the public and priv key - this->shanConn = std::make_shared(); + auto shanConn = std::make_shared(); + + // Init shanno-encrypted connection, and only then publish it so another + // task cannot pick up a connection that is not wrapped yet + shanConn->wrapConnection(this->conn, challenges->shanSendKey, + challenges->shanRecvKey); - // Init shanno-encrypted connection - this->shanConn->wrapConnection(this->conn, challenges->shanSendKey, - challenges->shanRecvKey); + std::scoped_lock lock(connMutex); + this->shanConn = shanConn; } void Session::connectWithRandomAp() { diff --git a/cspot/src/TrackReference.cpp b/cspot/src/TrackReference.cpp index b4fafcb5..203fe041 100644 --- a/cspot/src/TrackReference.cpp +++ b/cspot/src/TrackReference.cpp @@ -43,9 +43,9 @@ bool TrackReference::pbEncodeTrackList(pb_ostream_t* stream, // concurrent rebuild cannot reallocate the vector under us std::scoped_lock lock(*locked->mutex); auto& trackQueue = *locked->tracks; -#ifdef ESP_PLATFORM +#ifdef ESP_PLATFORM static TrackRef msg = TrackRef_init_zero; -#else +#else TrackRef msg = TrackRef_init_zero; #endif