8008aa175f
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>
173 lines
5.9 KiB
Diff
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
|
|
|