Files
aes67-ESP32-P4/components/spotify/patches/cspot/0001-mercury-reconnect-race-pr3.patch
T
bsncubed 8008aa175f cspot: patch in philippe44/cspot PR #3 (crash when a request races a reconnect)
MercurySession::reconnect() nulls conn/shanConn (for 5 s per failed
retry) while other tasks keep sending through them: a NULL dereference,
not a catchable exception. Our Spotify sessions are dropped by the
server and reconnect every ~6 minutes, so this race is hit regularly;
likely behind the sporadic resets during long sessions/OTA.
Patch applies cleanly to the pinned 3010349; builds, boots, confirmed.
Drop it once the fix is in the pinned cspot commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-25 20:17:01 +10:00

173 lines
5.9 KiB
Diff

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 <stdint.h> // for uint8_t
#include <memory> // for shared_ptr, unique_ptr
+#include <mutex> // for mutex
#include <string> // for string
#include <vector> // for vector
@@ -23,6 +24,11 @@ class Session {
std::shared_ptr<cspot::PlainConnection> conn;
std::shared_ptr<LoginBlob> 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<PlainConnection> 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<ShannonConnection> 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<std::underlying_type<RequestType>::type>(method),
sequenceIdBytes);
} catch (...) {
@@ -337,8 +364,21 @@ uint32_t MercurySession::requestAudioKey(const std::vector<uint8_t>& 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<ShannonConnection> 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<uint8_t>(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 <cstdint> // for uint8_t
#include <functional> // for __base
#include <memory> // for shared_ptr, unique_ptr, make_unique
+#include <mutex> // for scoped_lock
#include <random> // for default_random_engine, independent_bi...
#include <type_traits> // for remove_extent_t
#include <utility> // for move
@@ -33,7 +34,10 @@ Session::Session() {
Session::~Session() {}
void Session::connect(std::unique_ptr<cspot::PlainConnection> 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<cspot::PlainConnection> connection) {
CSPOT_LOG(debug, "Received shannon keys");
// Generates the public and priv key
- this->shanConn = std::make_shared<ShannonConnection>();
+ auto shanConn = std::make_shared<ShannonConnection>();
+
+ // 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