Skip to content

Commit 1ca412b

Browse files
committed
fix(transaction): distinguish enqueued transactions from pending claims
The runner clears the running slot before on_done removes the live registry entry. In that window manager.contains(id) is false while the live entry is still marked started, so rollback_claim_if_not_ enqueued and remove_for_destroy misclassified an already-executed transaction as a pre-enqueue claim: Cancel could report success after ApplyChanges had committed, and Destroy could remove the object before its terminal state was emitted. Add an enqueued flag to LiveTransaction, set atomically with the enqueue inside the live lock and kept until the entry is removed. Use it (instead of started + contains) in rollback_claim_if_ not_enqueued, remove_for_destroy, claim_expired and claim_still_ abandoned. Also drop the now-unused manager parameter from remove_for_destroy. Adds regression coverage for the finished-but-not-cleaned-up window. All 37 unit tests pass.
1 parent 766235b commit 1ca412b

4 files changed

Lines changed: 195 additions & 35 deletions

File tree

src/server.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,8 @@ impl Amo {
259259
claim_generation: String::new(),
260260
cancellation_id: None,
261261
started: false,
262+
// 尚未入队;begin 授权成功并入队后在 live 锁内置 true。
263+
enqueued: false,
262264
},
263265
);
264266
}

src/transaction/live.rs

Lines changed: 76 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ pub(crate) struct LiveTransaction {
9595
/// 被 zbus 取消,只有清扫器能回收并取消远程检查。
9696
pub(crate) cancellation_id: Option<String>,
9797
pub(crate) started: bool,
98+
/// 是否已入队:begin 授权成功后、`manager.enqueue` 成功时在 live 锁内
99+
/// 置 true(与入队原子可见),条目被 on_done 移除前一直保持。区分
100+
/// "已入队/运行中/已结束但尚未清理"的事务与"仅 claim 未入队"(授权
101+
/// 等待中)的事务:runner 清空 running 槽(`manager.contains` 暂时
102+
/// false)到 on_done 移除条目之间存在窗口,若只看 `started` +
103+
/// `contains`,Cancel/Destroy 会把已执行的事务误判为未入队的 claim——
104+
/// 取消报成功(但 ApplyChanges 已提交)或销毁移除尚未发终态信号的对象。
105+
pub(crate) enqueued: bool,
98106
}
99107

100108
/// `begin` 的启动声明(lease)守卫:在 live 锁内标记 `started` 后持有它,
@@ -228,17 +236,23 @@ pub(crate) struct ClaimRollback {
228236

229237
/// 在 live 锁内判定"已 claim 但未入队"(授权等待中)并回滚 claim:清
230238
/// started/claimed_at/claim_generation/cancellation_id,返回回滚结果与
231-
/// 带出的 cancellation_id(调用方锁外 `cancel_authorization`)。已入队/
232-
/// 运行中(manager 里有)返回 rolled_back=false(走 manager.cancel);
233-
/// 条目不存在返回 None(报 UnknownObject)。锁序 live→queue(与清扫器
234-
/// phase 3 的 claim_still_abandoned 一致)。
239+
/// 带出的 cancellation_id(调用方锁外 `cancel_authorization`)。
240+
///
241+
/// 以 `enqueued` 而非 `contains` 区分"未入队的 claim"与"已入队/运行中/收尾
242+
/// 中":runner 清空 running 槽到 on_done 移除条目之间 `contains` 会短暂
243+
/// false,此时已执行的事务若被误判为未入队 claim,Cancel 会报成功——但
244+
/// ApplyChanges 可能已提交。`enqueued` 在 live 锁内与入队原子置位,直到
245+
/// 条目移除才消失,无此窗口。条目不存在返回 None(报 UnknownObject)。
246+
/// 锁序 live→queue(与清扫器 phase 3 的 claim_still_abandoned 一致)。
235247
pub(crate) async fn rollback_claim_if_not_enqueued(
236248
live: &mut HashMap<u64, LiveTransaction>,
237249
manager: &TransactionManager,
238250
id: u64,
239251
) -> Option<ClaimRollback> {
240252
let t = live.get_mut(&id)?;
241-
if t.started && !manager.contains(id).await {
253+
// enqueued 为主判定;contains 作为防御(enqueued=false 时 begin 仍在
254+
// live 锁内入队,二者一致,此处不应命中,命中即编程错误)。
255+
if t.started && !t.enqueued && !manager.contains(id).await {
242256
let cid = t.cancellation_id.take();
243257
t.started = false;
244258
t.claimed_at = None;
@@ -259,19 +273,23 @@ pub(crate) async fn rollback_claim_if_not_enqueued(
259273

260274
/// 在 live 锁内判定 destroy 是否可行并移除条目:dormant(未启动)与
261275
/// 授权等待中(claimed-but-not-enqueued)可移除,后者带出 cancellation_id
262-
/// 供调用方锁外取消远程检查;已入队/运行中拒绝(Failed);条目不存在报
263-
/// UnknownObject。锁序 live→queue。
276+
/// 供调用方锁外取消远程检查;已入队/运行中/收尾中(`enqueued`)拒绝
277+
/// (Failed);条目不存在报 UnknownObject。锁序 live→queue。
278+
///
279+
/// 用 `enqueued` 而非 `contains` 判定:runner 清空 running 槽到 on_done
280+
/// 移除条目之间 `contains` 会短暂 false,此时若放行销毁会在 Finished
281+
/// 信号发出前移除对象(客户端永远收不到终态)。`enqueued` 在 live 锁内
282+
/// 与入队原子置位,直到条目移除才消失,无此窗口。
264283
pub(crate) async fn remove_for_destroy(
265284
live: &mut HashMap<u64, LiveTransaction>,
266-
manager: &TransactionManager,
267285
id: u64,
268286
) -> Result<Option<String>, zbus::fdo::Error> {
269287
let Some(t) = live.get_mut(&id) else {
270288
return Err(zbus::fdo::Error::UnknownObject(format!(
271289
"Transaction {id} no longer exists"
272290
)));
273291
};
274-
if t.started && manager.contains(id).await {
292+
if t.enqueued {
275293
return Err(zbus::fdo::Error::Failed(format!(
276294
"Transaction {id} already started"
277295
)));
@@ -318,8 +336,17 @@ pub(crate) async fn reclaim_dormant(
318336
let now = Instant::now();
319337

320338
// Phase 1:锁内快照候选(避免在 live 锁内做异步判定)。
321-
// 元组:(id, path, sender, started, created_at, claimed_at, dormant_since)。
322-
type Candidate = (u64, String, String, bool, Instant, Option<Instant>, Option<Instant>);
339+
// 元组:(id, path, sender, started, enqueued, created_at, claimed_at, dormant_since)。
340+
type Candidate = (
341+
u64,
342+
String,
343+
String,
344+
bool,
345+
bool,
346+
Instant,
347+
Option<Instant>,
348+
Option<Instant>,
349+
);
323350
let candidates: Vec<Candidate> = {
324351
let map = live.lock().await;
325352
map.iter()
@@ -329,6 +356,7 @@ pub(crate) async fn reclaim_dormant(
329356
t.path.clone(),
330357
t.sender.clone(),
331358
t.started,
359+
t.enqueued,
332360
t.created_at,
333361
t.claimed_at,
334362
t.dormant_since,
@@ -342,14 +370,17 @@ pub(crate) async fn reclaim_dormant(
342370
// (path, 快照时的 claimed_at):phase 3 用快照值做生成校验,防止
343371
// 误删"回滚后重新 claim"的新声明。
344372
let mut abandoned: Vec<(String, Option<Instant>)> = Vec::new();
345-
for (id, path, sender, started, created_at, claimed_at, dormant_since) in candidates {
373+
for (id, path, sender, started, enqueued, created_at, claimed_at, dormant_since) in
374+
candidates
375+
{
346376
if !started {
347377
if dormant_expired(dormant_since, created_at, now) {
348378
dormant_stale.push(path);
349379
}
350380
} else if claim_expired(
351381
&manager,
352382
&dbus,
383+
enqueued,
353384
id,
354385
&sender,
355386
claimed_at.unwrap_or(created_at),
@@ -387,7 +418,15 @@ pub(crate) async fn reclaim_dormant(
387418
let mut to_remove: Vec<u64> = Vec::new();
388419
for (path, snap_claimed_at) in &abandoned {
389420
if let Some((id, t)) = map.iter().find(|(_, t)| &t.path == path) {
390-
if claim_still_abandoned(&manager, t.claimed_at, *snap_claimed_at, *id).await {
421+
if claim_still_abandoned(
422+
&manager,
423+
t.claimed_at,
424+
*snap_claimed_at,
425+
t.enqueued,
426+
*id,
427+
)
428+
.await
429+
{
391430
to_remove.push(*id);
392431
}
393432
}
@@ -419,19 +458,27 @@ pub(crate) async fn reclaim_dormant(
419458
}
420459
}
421460

422-
/// 判定一个已 claim(started)但未入队的事务对象是否应被回收:创建者
423-
/// 连接已死(sender 锁定,操作永远无法继续),或 claim 超过 CLAIM_TIMEOUT
424-
/// 仍未入队(授权被放弃——即使创建者还连着,也视为超时,防止 claim 绕过
425-
/// 休眠/abandoned 回收被用来长期占槽)。已入队/运行中的事务即使创建者
426-
/// 断开也执行完,不回收。
461+
/// 判定一个已 claim(started)但未入队的事务对象是否应被回收:`enqueued`
462+
/// 的事务(已入队/运行中/已结束尚未清理)一律不回收——runner 清空 running
463+
/// 槽到 on_done 移除条目之间 `contains` 会短暂 false,此时若按 contains
464+
/// 判定,长任务超过 CLAIM_TIMEOUT 后会在收尾窗口被误回收(对象/终态信号
465+
/// 丢失)。创建者连接已死(sender 锁定,操作永远无法继续),或 claim 超过
466+
/// CLAIM_TIMEOUT 仍未入队(授权被放弃——即使创建者还连着,也视为超时,
467+
/// 防止 claim 绕过休眠/abandoned 回收被用来长期占槽)。已入队/运行中的
468+
/// 事务即使创建者断开也执行完,不回收。
427469
pub(crate) async fn claim_expired(
428470
manager: &TransactionManager,
429471
dbus: &zbus::fdo::DBusProxy<'_>,
472+
enqueued: bool,
430473
id: u64,
431474
sender: &str,
432475
claimed_at: Instant,
433476
now: Instant,
434477
) -> bool {
478+
if enqueued {
479+
// 已入队/运行中/收尾中:即使 contains 暂时 false 也不回收。
480+
return false;
481+
}
435482
if manager.contains(id).await {
436483
// 已入队/运行中:不回收。
437484
return false;
@@ -448,15 +495,23 @@ pub(crate) async fn claim_expired(
448495
}
449496

450497
/// 清扫器 phase 3 对单个 abandoned 候选的移除判定(在 live 锁内调用):
451-
/// ①生成校验——条目当前的 `claimed_at` 必须仍是快照时的值,期间被回滚后
452-
/// 重新 claim 的新声明(重试)不删;②事务仍未入队才移除(begin 在 live
453-
/// 锁内完成入队,与这里互斥,故这里持锁复查时结果稳定)。
498+
/// ①`enqueued` 的事务(已入队/运行中/收尾中)不删——runner 清空 running
499+
/// 槽到 on_done 移除条目之间 `contains` 短暂 false,若按 contains 判定会
500+
/// 误删尚未发终态信号的对象;②生成校验——条目当前的 `claimed_at` 必须仍
501+
/// 是快照时的值,期间被回滚后重新 claim 的新声明(重试)不删;③事务仍
502+
/// 未入队才移除(begin 在 live 锁内完成入队,与这里互斥,故这里持锁复查
503+
/// 时结果稳定)。
454504
pub(crate) async fn claim_still_abandoned(
455505
manager: &TransactionManager,
456506
entry_claimed_at: Option<Instant>,
457507
snap_claimed_at: Option<Instant>,
508+
enqueued: bool,
458509
id: u64,
459510
) -> bool {
511+
if enqueued {
512+
// 已入队/运行中/收尾中:不删。
513+
return false;
514+
}
460515
if entry_claimed_at != snap_claimed_at {
461516
// 回滚后重新 claim(claimed_at 变了):新声明,不删。
462517
return false;

src/transaction/object.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -333,14 +333,27 @@ impl TransactionObject {
333333
// 可回收后 begin 仍入队"的窗口(否则已入队/运行中的事务对象消失,
334334
// 队列中的包操作无法取消)。
335335
let enqueue_result = {
336-
let live = self.live.lock().await;
336+
let mut live = self.live.lock().await;
337337
// 授权等待期间对象可能被并发 Cancel 回滚(started=false)、
338338
// Destroy 移除(条目缺失)、或 Cancel 后立即重试(claim 代际
339339
// 被新 claim 替换):中止入队,不执行已取消的操作。
340340
check_claim_still_active(&live, self.id, &claim_generation)?;
341-
self.manager
341+
let result = self
342+
.manager
342343
.enqueue(ctxt_owned, self.id, role, caller, uid, task, on_done)
343-
.await
344+
.await;
345+
// 入队成功:在 live 锁内标记 enqueued,与 manager 入队原子可见
346+
// (同一临界区)。Cancel/Destroy/清扫器据此区分"已入队/运行中/
347+
// 收尾中"与"仅 claim 未入队"——runner 清空 running 槽到 on_done
348+
// 移除条目之间 manager.contains 短暂 false,enqueued 保持 true
349+
// 直到条目移除,杜绝把已执行的事务误判为未入队 claim(取消报
350+
// 成功但操作已提交,或销毁移除未发终态信号的对象)。
351+
if result.is_ok()
352+
&& let Some(entry) = live.get_mut(&self.id)
353+
{
354+
entry.enqueued = true;
355+
}
356+
result
344357
};
345358
match enqueue_result {
346359
Ok(_) => claim.commit(),
@@ -618,7 +631,7 @@ impl TransactionObject {
618631
// 先移除注册表条目,再移除 D-Bus 对象。
619632
let cancel_id = {
620633
let mut live = self.live.lock().await;
621-
remove_for_destroy(&mut live, &self.manager, self.id).await?
634+
remove_for_destroy(&mut live, self.id).await?
622635
};
623636
if let Some(cid) = cancel_id {
624637
crate::auth::cancel_authorization(conn, &cid).await;

0 commit comments

Comments
 (0)