[PR #776] [CLOSED] Improvements in librespot-core #1073

New issue

Closed

opened 2026-02-27 20:01:03 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:01:03 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/librespot-org/librespot/pull/776
Author: @Johannesd3
Created: 5/28/2021
Status: ❌ Closed

Base: dev ← Head: remove-weak-refs

📝 Commits (8)

17e6718 Remove weak refs in session components
2d71de9 Make them Copy
5b6b010 Use MercuryFuture for subscriptions
f874df4 Split TcpStream for connection immediately
d037992 Remove unused allocation
73c82a0 Add macro to create packets
917234d Changes suggested by @plietar
fc754c5 Remove MercurySender

📊 Changes

20 files changed (+567 additions, -346 deletions)

View changed files

📝 Cargo.lock (+0 -1)
📝 audio/src/fetch/receive.rs (+13 -12)
📝 audio/src/lib.rs (+1 -1)
📝 connect/src/spirc.rs (+51 -42)
➖ core/src/component.rs (+0 -37)
📝 core/src/connection/codec.rs (+33 -21)
📝 core/src/connection/handshake.rs (+32 -24)
📝 core/src/connection/mod.rs (+14 -6)
📝 core/src/lib.rs (+3 -8)
➖ core/src/mercury/sender.rs (+0 -53)
📝 core/src/session.rs (+45 -33)
📝 core/src/session/audio_key.rs (+14 -13)
📝 core/src/session/channel.rs (+6 -6)
➕ core/src/session/component.rs (+46 -0)
📝 core/src/session/mercury/mod.rs (+75 -59)
📝 core/src/session/mercury/types.rs (+41 -21)
📝 core/src/util.rs (+186 -0)
📝 metadata/Cargo.toml (+0 -1)
📝 metadata/src/cover.rs (+7 -6)
📝 metadata/src/lib.rs (+0 -2)

📄 Description

The so-called components AudioKeyManager, ChannelManager and MercuryManager all possess a weak reference to SessionInternal. I'm not sure why they were introduced around 855a7e87a7 (@plietar do you remember?), but I noticed that it's possible to replace them with a lifetime-based approach:

struct MercuryManager<'a>(&'a SessionInternal)

The components would be plain references to SessionInternal. They just have some additional methods, and that's it.

MercuryManagerInner & co would be saved behind a mutex in SessionInternal:

github.com/librespot-org/librespot@17e671814b/core/src/session.rs (L55-L57)

The only difficulty was in spirc, but due to async/await it was possible to generate a self-referential future which owns a Session and at the same contains a reference to it inside of the MercurySender.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/librespot-org/librespot/pull/776 **Author:** [@Johannesd3](https://github.com/Johannesd3) **Created:** 5/28/2021 **Status:** ❌ Closed **Base:** `dev` ← **Head:** `remove-weak-refs` --- ### 📝 Commits (8) - [`17e6718`](https://github.com/librespot-org/librespot/commit/17e671814b14d98b354973ebe623abe693296c92) Remove weak refs in session components - [`2d71de9`](https://github.com/librespot-org/librespot/commit/2d71de94b1a95de77db2f764372f41531a26b6a6) Make them `Copy` - [`5b6b010`](https://github.com/librespot-org/librespot/commit/5b6b010ce6add2631e34ee6c07d0cebc0ad197d9) Use `MercuryFuture` for subscriptions - [`f874df4`](https://github.com/librespot-org/librespot/commit/f874df453d24ff705437fa32b5914ab2d2198bb8) Split TcpStream for connection immediately - [`d037992`](https://github.com/librespot-org/librespot/commit/d037992666ddddf91838c87b75c4be5c8f3dac1f) Remove unused allocation - [`73c82a0`](https://github.com/librespot-org/librespot/commit/73c82a0430c0fba4aa268de7ad578a5f3e2dd097) Add macro to create packets - [`917234d`](https://github.com/librespot-org/librespot/commit/917234d4135da2c3db9cb5e46abf881e25604876) Changes suggested by @plietar - [`fc754c5`](https://github.com/librespot-org/librespot/commit/fc754c58fdc840e47ed4cbd78b85f2fd5a279ac4) Remove MercurySender ### 📊 Changes **20 files changed** (+567 additions, -346 deletions) <details> <summary>View changed files</summary> 📝 `Cargo.lock` (+0 -1) 📝 `audio/src/fetch/receive.rs` (+13 -12) 📝 `audio/src/lib.rs` (+1 -1) 📝 `connect/src/spirc.rs` (+51 -42) ➖ `core/src/component.rs` (+0 -37) 📝 `core/src/connection/codec.rs` (+33 -21) 📝 `core/src/connection/handshake.rs` (+32 -24) 📝 `core/src/connection/mod.rs` (+14 -6) 📝 `core/src/lib.rs` (+3 -8) ➖ `core/src/mercury/sender.rs` (+0 -53) 📝 `core/src/session.rs` (+45 -33) 📝 `core/src/session/audio_key.rs` (+14 -13) 📝 `core/src/session/channel.rs` (+6 -6) ➕ `core/src/session/component.rs` (+46 -0) 📝 `core/src/session/mercury/mod.rs` (+75 -59) 📝 `core/src/session/mercury/types.rs` (+41 -21) 📝 `core/src/util.rs` (+186 -0) 📝 `metadata/Cargo.toml` (+0 -1) 📝 `metadata/src/cover.rs` (+7 -6) 📝 `metadata/src/lib.rs` (+0 -2) </details> ### 📄 Description The so-called components `AudioKeyManager`, `ChannelManager` and `MercuryManager` all possess a weak reference to `SessionInternal`. I'm not sure why they were introduced around 855a7e87a77506d6c9339b900becbd3c32e085dd (@plietar do you remember?), but I noticed that it's possible to replace them with a lifetime-based approach: ```rust struct MercuryManager<'a>(&'a SessionInternal) ``` The components would be plain references to `SessionInternal`. They just have some additional methods, and that's it. `MercuryManagerInner` & co would be saved behind a mutex in `SessionInternal`: https://github.com/librespot-org/librespot/blob/17e671814b14d98b354973ebe623abe693296c92/core/src/session.rs#L55-L57 The only difficulty was in `spirc`, but due to `async`/`await` it was possible to generate a self-referential future which owns a `Session` and at the same contains a reference to it inside of the `MercurySender`. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

kerem

2026-02-27 20:01:03 +03:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/librespot#1073

No description provided.

Rows
Columns