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
parent f0fe53d5d1
commit 8008aa175f
1 changed files with 172 additions and 0 deletions
@@ -0,0 +1,172 @@
+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
+