Skip to content

Commit 4d9dbfd

Browse files
committed
fix: align terminal unit contracts with the imported behavior
- Provider install/removal admission moves to request time (revision bumps before the commit turn): a newer transaction supersedes stale staged bytes, while the per-module turn keeps commits and directory removal in request order. - The remote keeps its exact optimistic position contract: positionTicks is the raw pinned value and pause/unpause/stop pin the current base, so an optimistic seek target survives stale snapshots; the predicted clock remains for media-session consumers. - The performance monitor test registers an observer, matching the sample-only-while-observed lifecycle.
1 parent 40de096 commit 4d9dbfd

4 files changed

Lines changed: 20 additions & 12 deletions

File tree

‎src/app/RemoteTargetsController.cpp‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -643,14 +643,11 @@ void RemoteTargetsController::beginOptimistic(const QVariantMap& command)
643643
const auto name = command.value("action").toString();
644644
m_ackSequence.reset();
645645
if (name == "pause" || name == "unpause" || name == "stop") {
646-
// Capture the running clock before the transport changes state: a
646+
// Capture the current position before the transport changes state: a
647647
// stale poll must neither rewind it nor restart an optimistic
648-
// unpause's clock, and playback advances even from a zero position.
649-
if (!m_position.isEmpty()) {
650-
const QString captured = QString::number(predictedPositionTicks());
651-
m_optimistic.insert("positionTicks", captured);
652-
m_position = captured;
653-
}
648+
// unpause's clock. An optimistic seek target is already exact.
649+
if (!m_position.isEmpty())
650+
m_optimistic.insert("positionTicks", m_position);
654651
m_optimistic.insert("state", name == "pause" ? "paused" : name == "unpause" ? "playing" : "stopped");
655652
} else if (name == "seek")
656653
m_optimistic.insert("positionTicks", command.value("positionTicks"));

‎src/app/RemoteTargetsController.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,13 @@ class RemoteTargetsController final : public QObject {
6464
return m_target;
6565
}
6666
QVariantMap state() const;
67-
// The running clock: the last accepted position advanced by elapsed time
68-
// while the target plays. Zero is a real position, not a frozen clock.
67+
// The running clock for media-session consumers: the last accepted
68+
// position advanced by elapsed time while the target plays. Zero is a
69+
// real position, not a frozen clock.
6970
qint64 predictedPositionTicks() const;
7071
QString positionTicks() const
7172
{
72-
return m_position.isEmpty() ? m_position : QString::number(predictedPositionTicks());
73+
return m_position;
7374
}
7475
QString runtimeTicks() const
7576
{

‎src/provider/ProviderRegistry.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,10 @@ QCoro::Task<void> ProviderRegistry::install(
932932
if (root.isEmpty())
933933
throw std::runtime_error("installs_disabled");
934934
const QString id = package.manifest.id;
935+
// Admission spans the request: a newer install or removal supersedes this
936+
// one before its bytes can commit, while the turn below keeps commits and
937+
// directory removal in request order.
938+
const quint64 revision = ++m_installRevisions[id];
935939
QPromise<void> turn;
936940
turn.start();
937941
const QFuture<void> previous = std::exchange(m_installTails[id], turn.future());
@@ -943,7 +947,6 @@ QCoro::Task<void> ProviderRegistry::install(
943947
if (const auto *existing = module(id);
944948
existing && ProviderPackage::compareVersions(package.manifest.version, existing->manifest.version) <= 0)
945949
throw std::runtime_error("install_version_not_newer");
946-
const quint64 revision = ++m_installRevisions[id];
947950
QPointer<ProviderRegistry> guard(this);
948951
// Only inert files are written in the worker. Admission and activation
949952
// share the registry thread, so a concurrent replacement cannot slip
@@ -973,14 +976,17 @@ QCoro::Task<void> ProviderRegistry::install(
973976

974977
QCoro::Task<void> ProviderRegistry::uninstall(QString moduleId)
975978
{
979+
// A removal request retires older pending installs immediately, while the
980+
// turn keeps its directory removal behind any earlier commit and ahead of
981+
// any newer install's activation.
982+
++m_installRevisions[moduleId];
976983
QPromise<void> turn;
977984
turn.start();
978985
const QFuture<void> previous = std::exchange(m_installTails[moduleId], turn.future());
979986
const auto releaseTurn = qScopeGuard([&turn] { turn.finish(); });
980987
if (previous.isValid())
981988
co_await qCoro(previous).result();
982989
clearGrants(moduleId);
983-
++m_installRevisions[moduleId];
984990
const ProviderModule *existing = module(moduleId);
985991
if (!existing || (existing->bundled && !existing->overridesBundled) || existing->native)
986992
co_return;

‎tests/diagnostics/SystemPerformanceMonitorTest.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ SPOOL_TEST_MAIN("system-performance-monitor")
2222
{
2323
QCoreApplication app(argc, argv);
2424
Spool::SystemPerformanceMonitor monitor;
25+
// Sampling runs only while an observer (the diagnostics overlay) is
26+
// registered; the bare monitor holds no thread or counters of its own.
27+
QObject overlayObserver;
28+
monitor.observe(&overlayObserver);
2529
std::atomic<qint64> fakeAudioDecodeTimeNs { 0 };
2630
monitor.setAudioDecodeCpuTimeProvider(
2731
[&fakeAudioDecodeTimeNs] { return fakeAudioDecodeTimeNs.fetch_add(1'000'000) + 1'000'000; });

0 commit comments

Comments
 (0)