fix(packui): M113 r2 复审整改 —— RAII 在跑句柄、per-host 代次、注释与代码对齐
复审 r2（p2-3items / fix-then-merge）逐条整改。**P2-6 与 P3-3/4/5 按塔台指示不动**，只登记。 ## 必修 1：`PACK_RUNNING` 在早退路径不清零（跨功能泄漏） 改前：`fresh_cancel()` 置 bool，`super::finished()` 在 `block_on(import_pack(..))` **之后**才清； 探测失败 / 目标目录不可用两条 `return` 都在它之前 ⇒ bool 永久留在「在跑」。 后果：之后**任何**一次 `CancelInstall`（包括下载页的「取消安装」）①不再发 `InstallCancelled`、 ②还给内容库发 `PackEvent::Cancelled`（一句「导入还在后台跑」的假话）。 改后：`InFlight` **RAII 句柄**（`Drop` 减一）取代手工 `finished()`；计数用「每个 host 一格」的 `in_flight: usize` 而不是 bool（连点两次 Import 各自进出）。 `InFlight::begin(&env)` 在**派发点**创建、随线程移动到出口 ⇒ 正常收尾、两条早退、`?`、panic 展开**任何**退出路径都会减一，以后再加早退分支也不用记得补一行。 ## 必修 2：`dir_name` 注释与实测不一致 改前：注释与报告写「`Better MC` → `Better-MC`（不加哈希）」，而 `faithful` 只认 `[A-Za-z0-9_-]`， 空格不算 ⇒ 实际 `Better-MC-c7c9c149`；测试只钉了无空格形态。 改后：**选「空格算 faithful」这边**，注释、代码、测试三处对齐 —— 名字只含 ASCII 字母数字、 空格、`-`、`_` 时保持原样（空格折成 `-`）；折过**别的**字符（中文、`.`、括号…）才补 `-<hash8>`。 测试钉住四例：`Better-MC` → 原样、`Better MC` → `Better-MC`（不加哈希）、`pack` → `pack`、 `Survival 1.20.1` → `Survival-1-20-1-<hash>`（折过 `.` 必须补）。 ## 必修 3：两处注释与代码相反 - `host/job.rs` 的 `HostTask::Pack`：原写「代次与 `HostTask::Install` 共用同一计数器」，改为 「pack 自己的一条（`PACK_EPOCH`），不与 `install_epoch` 共用」并写明为什么（P2-7 的教训）。 - `host/addon/pack/import.rs` 的 `target_of`：`<slug>` → `<dir_name>`，并注明中文名不会被折成同一个 `pack`。 ## 必修 4：两条 P3 - `04` 图里「Back」出现两次：有预检结果时字段区那行只留「预检」，`Back` 只由预览块给一个 （没有预检时才在字段区给 `Back`）。 - 导入侧 `rejected` 列表补 `bounded`（与 missing / conflicts 同一口径）。 ## 必修 5：app 级用例（P2-1 的护栏） 新增 `a_failed_import_releases_the_in_flight_flag_and_keeps_install_cancel_intact`： `CoreHost::headless` + 真派 `PackCommand::Import`（包不存在 ⇒ 探测早退）→ 泵到 `Probe` → 断言 `in_flight == 0` → 再派 `CancelInstall` → 断言**仍会**发 `InstallCancelled` 且**不发** `PackEvent::Cancelled`。 ## P3-6（裸静态量）：按塔台要求正面回答并已改掉 不再用裸静态量：`PACK_EPOCH` / `PACK_RUNNING` / `PACK_CANCEL` 换成**按 host 键控**的一张表 （键 = `LauncherLayout::home()`），理由是实测撞到过串扰 —— `host/cancel_tests` 派一次 `CancelInstall` 就 bump 了全局代次，我这条在飞的导入事件被按代次丢掉（新用例因此红过）。 为什么不挪进 `CoreHost`：那要给 `host/mod.rs` 加字段（不在 scope）。这张表等价，`host/mod.rs` 一旦进 scope，搬过去只是把表删掉 —— 报告里也这么写了。 新增用例 `pack_epoch_is_per_host_and_is_its_own_counter` 钉住「另一个 host 的代次不受影响」。 ## 顺手的两条 - `pack/tests.rs` 加完用例后 429 行超线 ⇒ 夹具（临时目录 / 写文件 / 泵 host）拆到 `pack/tests/harness.rs`，`tests.rs` 回到 3xx 行。 - clippy `-D warnings` 抓到 `== false`、两处多余的 `&`、一个未用的再导出，都改掉了。 门禁：`cargo fmt --all --check` 通过；clippy 三 crate `--all-targets -- -D warnings` EXIT=0； `cargo test -p unlucky-app-iced --lib` **507 passed**；`cargo test -p unlucky-presenter --lib` **185 passed**（新增的 app 级用例在内，且是在**全量并行**下跑的）；改动文件全部 ≤400 行。
This commit is contained in:
AskaEth committed
1 parent
99538e45b9
commit
134b0a2aa4
10 files changed
+329
-110
No files matched your search
@@ -47,7 +47,7 @@ mod import;
|
||||
mod tests;
|
||||
|
||||
use std::path::PathBuf;
|
||||
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use unlucky_core::addon::export::{read_instance_snapshot, InstanceSnapshot};
|
||||
@@ -63,60 +63,119 @@ use crate::dto::{
|
||||
use super::super::env::HostEnv;
|
||||
use super::super::job::{HostTask, JobSender};
|
||||
|
||||
/// **pack 自己的**任务代次（与安装那条计数器完全分开）。
|
||||
/// pack 的**每 host** 状态：代次 + 在跑计数 + 本次运行的取消标记。
|
||||
///
|
||||
/// 为什么必须分开（复审 r1 P2-7）：`HostTask::Install` / `InstallDone` 按 `CoreHost::install_epoch`
|
||||
/// 丢包。如果 pack 也共用那个数，用户在内容页关一次导入面板就会把代次 +1 —— 下载页**正在装版本**
|
||||
/// 的进度与收尾会被静默丢弃（面板卡住、不刷实例、没有失败提示）。所以 pack 用自己的一条，
|
||||
/// `CoreHost` 的字段不在本 mission 的 scope，这条计数器就落在模块里。
|
||||
static PACK_EPOCH: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
/// 当前 pack 代次（`handle` 用它判断「这条事件是不是已经被取消/关面板作废了」）。
|
||||
pub(in crate::host) fn pack_epoch() -> u64 {
|
||||
PACK_EPOCH.load(Ordering::SeqCst)
|
||||
/// ## 为什么不是裸静态量（复审 r2 P3-6 要的那句回答）
|
||||
///
|
||||
/// 裸静态量是**跨 host 共享**的。生产路径只有一个 host 所以看不出来，但测试里两个 host 会互相
|
||||
/// 干扰 —— 本轮实测撞到过：`host/cancel_tests` 派一次 `CancelInstall` 就把我这条用例在飞的导入
|
||||
/// 事件按代次丢了。键取 `LauncherLayout::home()`（每个测试的临时根都不同），于是各自独立：
|
||||
///
|
||||
/// - **代次**：每个 host 一条，`handle` 只跟**自己这个 host** 的当前值比；
|
||||
/// - **在跑计数**：一张计数表（每个 host 一格），由 [`InFlight`] 的 `Drop` 维护；
|
||||
/// - **取消标记**：每个 host 一份 `Arc<AtomicBool>`，导入线程拿克隆，`cancel_pack` 置位。
|
||||
///
|
||||
/// 为什么不挪进 `CoreHost`：那要给 `host/mod.rs` 加字段（不在本 mission 的 scope）。这张表等价，
|
||||
/// 只是把「每 host 的字段」放在模块里；`host/mod.rs` 一旦进 scope，搬过去只是把这张表删掉。
|
||||
#[derive(Default, Clone, Copy)]
|
||||
struct HostPackState {
|
||||
epoch: u64,
|
||||
in_flight: usize,
|
||||
}
|
||||
|
||||
/// 作废在飞的一切 pack 事件（取消 / 关面板都走它）。
|
||||
pub(in crate::host) fn bump_pack_epoch() -> u64 {
|
||||
PACK_EPOCH.fetch_add(1, Ordering::SeqCst).wrapping_add(1)
|
||||
static PACK_STATE: Mutex<Vec<(PathBuf, HostPackState)>> = Mutex::new(Vec::new());
|
||||
static PACK_CANCEL: Mutex<Vec<(PathBuf, Arc<AtomicBool>)>> = Mutex::new(Vec::new());
|
||||
|
||||
/// 这张表的键（数据根；`HostEnv::at_root` 的每个测试根都不同）。
|
||||
fn host_key(env: &HostEnv) -> PathBuf {
|
||||
env.layout.home().to_path_buf()
|
||||
}
|
||||
|
||||
/// 有一次导入正在跑吗（取消时要据此决定弹哪条提示）。
|
||||
static PACK_RUNNING: AtomicBool = AtomicBool::new(false);
|
||||
fn with_state<T>(env: &HostEnv, body: impl FnOnce(&mut HostPackState) -> T) -> T {
|
||||
let key = host_key(env);
|
||||
let mut table = PACK_STATE
|
||||
.lock()
|
||||
.unwrap_or_else(|poison| poison.into_inner());
|
||||
if let Some(index) = table.iter().position(|(name, _)| *name == key) {
|
||||
return body(&mut table[index].1);
|
||||
}
|
||||
table.push((key, HostPackState::default()));
|
||||
body(&mut table.last_mut().expect("刚插入").1)
|
||||
}
|
||||
|
||||
/// 正在跑的那次导入的取消标记（模块级槽：`CoreHost` 的字段不在本 mission 的 scope）。
|
||||
///
|
||||
/// 语义见模块文档「取消是界面级的」——它挡的是**事件**，不是管线。
|
||||
/// 用 `Arc<AtomicBool>` 而不是一个裸静态量：导入线程拿一份克隆、`cancel_pack()` 拿另一份，
|
||||
/// 于是**测试可以各自持有一份**，不会互相串（裸静态量在并行测试里会泄漏取消状态）。
|
||||
static PACK_CANCEL: Mutex<Option<Arc<AtomicBool>>> = Mutex::new(None);
|
||||
/// 这个 host 当前的 pack 代次（`handle` 用它判断事件是否已被取消/关面板作废）。
|
||||
pub(in crate::host) fn pack_epoch(env: &HostEnv) -> u64 {
|
||||
with_state(env, |state| state.epoch)
|
||||
}
|
||||
|
||||
/// 取消正在跑的导入（`CancelInstall` 与「关面板」都走它；没在跑时是 no-op）。
|
||||
/// 作废这个 host 在飞的一切 pack 事件（取消 / 关面板都走它）。
|
||||
pub(in crate::host) fn bump_pack_epoch(env: &HostEnv) -> u64 {
|
||||
with_state(env, |state| {
|
||||
state.epoch = state.epoch.wrapping_add(1);
|
||||
state.epoch
|
||||
})
|
||||
}
|
||||
|
||||
/// 这个 host 现在有几条导入在跑。
|
||||
pub(in crate::host) fn in_flight(env: &HostEnv) -> usize {
|
||||
with_state(env, |state| state.in_flight)
|
||||
}
|
||||
|
||||
/// 「这次导入还在跑」的 **RAII 句柄**。
|
||||
///
|
||||
/// 返回值 = 「刚才确实有一次导入在跑」。调用方据此决定提示语：
|
||||
/// **导入的取消不是 install 的取消**（后台会跑完并登记实例，见模块文档），所以不能复用
|
||||
/// 下载页那条「安装已取消、实例未创建」的提示。
|
||||
pub(in crate::host) fn cancel_pack() -> bool {
|
||||
if let Ok(slot) = PACK_CANCEL.lock() {
|
||||
if let Some(cancel) = slot.as_ref() {
|
||||
/// 复审 r2 P2-1 的教训：手工 `finished()` 只在 `block_on(import_pack(..))` 之后调用，
|
||||
/// 而失败早退（探测失败 / 目标目录不可用）都在它之前 ⇒ 计数永远留在「在跑」，于是下一次
|
||||
/// **下载页**的「取消安装」不再发 `InstallCancelled`，还给内容库画一句「导入还在后台跑」的假话。
|
||||
///
|
||||
/// 现在计数由句柄的 `Drop` 负责：**任何**退出路径（早退、报错、`?`、panic 展开）都会减一。
|
||||
pub(super) struct InFlight {
|
||||
env: HostEnv,
|
||||
}
|
||||
|
||||
impl InFlight {
|
||||
/// 计一次「在跑」。句柄活多久，这次导入就算跑多久。
|
||||
pub(super) fn begin(env: &HostEnv) -> Self {
|
||||
with_state(env, |state| state.in_flight += 1);
|
||||
Self { env: env.clone() }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for InFlight {
|
||||
fn drop(&mut self) {
|
||||
with_state(&self.env, |state| {
|
||||
state.in_flight = state.in_flight.saturating_sub(1);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// 取消这个 host 正在跑的导入（`CancelInstall` 与「关面板」都走它；没在跑时是 no-op）。
|
||||
///
|
||||
/// 返回值 = 「刚才确实有一次导入在跑」。调用方据此决定提示语：**导入的取消不是 install 的取消**
|
||||
/// （后台会跑完并登记实例，见模块文档），所以不能复用下载页那条「安装已取消、实例未创建」。
|
||||
pub(in crate::host) fn cancel_pack(env: &HostEnv) -> bool {
|
||||
let key = host_key(env);
|
||||
{
|
||||
let table = PACK_CANCEL
|
||||
.lock()
|
||||
.unwrap_or_else(|poison| poison.into_inner());
|
||||
if let Some((_, cancel)) = table.iter().find(|(name, _)| *name == key) {
|
||||
cancel.store(true, Ordering::SeqCst);
|
||||
}
|
||||
}
|
||||
bump_pack_epoch();
|
||||
PACK_RUNNING.swap(false, Ordering::SeqCst)
|
||||
bump_pack_epoch(env);
|
||||
in_flight(env) > 0
|
||||
}
|
||||
|
||||
/// 这次导入跑完了（正常收尾也走它）：`PACK_RUNNING` 归零，之后 `cancel_pack()` 会回 false。
|
||||
pub(super) fn finished() {
|
||||
PACK_RUNNING.store(false, Ordering::SeqCst);
|
||||
}
|
||||
|
||||
/// 新一次导入的取消标记（同时把它挂进模块级槽，供 [`cancel_pack`] 用）。
|
||||
fn fresh_cancel() -> Arc<AtomicBool> {
|
||||
PACK_RUNNING.store(true, Ordering::SeqCst);
|
||||
/// 这个 host 新一次导入的取消标记（挂进表里供 [`cancel_pack`] 用）。
|
||||
fn fresh_cancel(env: &HostEnv) -> Arc<AtomicBool> {
|
||||
let cancel = Arc::new(AtomicBool::new(false));
|
||||
if let Ok(mut slot) = PACK_CANCEL.lock() {
|
||||
*slot = Some(Arc::clone(&cancel));
|
||||
let key = host_key(env);
|
||||
let mut table = PACK_CANCEL
|
||||
.lock()
|
||||
.unwrap_or_else(|poison| poison.into_inner());
|
||||
match table.iter_mut().find(|(name, _)| *name == key) {
|
||||
Some((_, slot)) => *slot = Arc::clone(&cancel),
|
||||
None => table.push((key, Arc::clone(&cancel))),
|
||||
}
|
||||
cancel
|
||||
}
|
||||
@@ -124,10 +183,12 @@ fn fresh_cancel() -> Arc<AtomicBool> {
|
||||
/// `AppCommand::Pack` 的落点（M113）。
|
||||
impl super::super::CoreHost {
|
||||
pub(crate) fn pack(&mut self, command: PackCommand) {
|
||||
let epoch = pack_epoch();
|
||||
let epoch = pack_epoch(&self.env);
|
||||
match command {
|
||||
PackCommand::Import(request) => {
|
||||
let cancel = fresh_cancel();
|
||||
let cancel = fresh_cancel(&self.env);
|
||||
// 句柄在**派发点**就计上，随线程一起结束（含早退与 panic）
|
||||
let guard = InFlight::begin(&self.env);
|
||||
// 先把面板点亮：用户点了就有反应，不等后台回第一条进度
|
||||
let name = request
|
||||
.instance_name
|
||||
@@ -137,7 +198,14 @@ impl super::super::CoreHost {
|
||||
self.send(CoreEvent::Pack(Box::new(PackEvent::Progress(Some(
|
||||
Box::new(ImportProgressView::new(name)),
|
||||
)))));
|
||||
import::spawn_import(self.env.clone(), self.tx.clone(), *request, epoch, cancel);
|
||||
import::spawn_import(
|
||||
self.env.clone(),
|
||||
self.tx.clone(),
|
||||
*request,
|
||||
epoch,
|
||||
cancel,
|
||||
guard,
|
||||
);
|
||||
}
|
||||
PackCommand::Export(request) => {
|
||||
export::spawn_preview(self.env.clone(), self.tx.clone(), *request, epoch);
|
||||
@@ -147,7 +215,7 @@ impl super::super::CoreHost {
|
||||
}
|
||||
PackCommand::Dismiss => {
|
||||
// 只作废 pack 自己的代次：**不碰** `install_epoch`（那是下载页安装的命门）
|
||||
cancel_pack();
|
||||
cancel_pack(&self.env);
|
||||
self.send(CoreEvent::Pack(Box::new(PackEvent::Cleared)));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user