Detach Background Tasks Directly
This commit is contained in:
@@ -440,6 +440,18 @@ octal-escaped/non-ASCII quoted paths) before committing to the swap.
|
||||
|
||||
## 6. Remove the `tasks: Vec<Task<...>>` + `push_task` boilerplate — use `Task::detach()`
|
||||
|
||||
> **Status: done.** Removed the `tasks` field and `push_task` from all six
|
||||
> stores (`backend.rs`, `checkouts.rs`, `local_repos.rs`, `profile.rs`,
|
||||
> `repo.rs`, `repo_list.rs`); every call site now ends in `.detach()`
|
||||
> instead. As with §14, most `cx.spawn` sites lost their type-inference
|
||||
> anchor and needed an explicit `let task: Task<Result<(), Error>> = ...`
|
||||
> (or `gpui::Task<...>` where `Task` wasn't imported) before `.detach()`.
|
||||
> A few closures that captured a variable also named `task` (the awaited
|
||||
> inner task) were given a distinct outer name (`notify_task`, `publish`,
|
||||
> `fetch`, `sync`) to avoid a confusing shadow. `cargo check --workspace`
|
||||
> and `cargo test -p signed_state` (24 tests) / `cargo test -p workspace`
|
||||
> (14 tests) all pass.
|
||||
|
||||
Verified against the actual pinned GPUI revision
|
||||
(`~/.cargo/git/checkouts/zed-a70e2ad075855582/1870e26/crates/scheduler/src/executor.rs:375-573`
|
||||
and `crates/gpui/src/executor.rs:32-63`).
|
||||
@@ -952,6 +964,14 @@ entirely disjoint observers.
|
||||
|
||||
## 14. `crates/workspace` has the same task-list pattern as §6 — and there it's an actual bug
|
||||
|
||||
> **Status: done.** The `tasks` field and all 17 push sites were removed from
|
||||
> `RepoDetailView`, `NewPullRequestView`, `CommitDiffView` and
|
||||
> `PullRequestDetailView`, replaced with `.detach()`. `cargo check -p workspace`
|
||||
> and `cargo test -p workspace` (14 tests) pass. Removing the field cost each
|
||||
> `cx.spawn`/`cx.spawn_in` call site its type-inference anchor, so every
|
||||
> remaining spawn site needed an explicit `let task: gpui::Task<Result<(), ...>> = ...`
|
||||
> annotation — expect the same when doing §6's `signed_state` half.
|
||||
|
||||
§6 covers `signed_state`'s 6 stores, where the unpruned-`Vec<Task>` pattern
|
||||
is a style/complexity concern with no observed failure, because
|
||||
`push_task` always pruned before pushing. `crates/workspace` has the exact
|
||||
@@ -1219,14 +1239,24 @@ method.
|
||||
|
||||
## Action plan, in order of risk/reward
|
||||
|
||||
1. **Delete the fetch/sync dedup cache** (§3). Pure removal, no behavior
|
||||
1. ✅ **Delete the fetch/sync dedup cache** (§3). Pure removal, no behavior
|
||||
change for the intended usage pattern (each call site already has, or
|
||||
trivially gets, its own guard). Lowest risk, do first.
|
||||
2. **Remove the `tasks: Vec<Task<...>>` + `push_task` boilerplate**, in
|
||||
|
||||
Done: removed `recent_fetches`/`fetch_recently_started`/`fetch_fingerprint`/
|
||||
`FETCH_DEDUP_WINDOW` and the now-unused `DefaultHasher`/`Hash`/`Hasher`/
|
||||
`Instant` imports from `signed_state/src/backend.rs`. `connect_repo_relays`
|
||||
and `sync_bootstrap` no longer fingerprint or gate on a cache; callers keep
|
||||
their own guards (`RepoStore::repo_relays`, one-shot construction-time call
|
||||
in `RepoListStore::subscribe_remote`). `cargo check --workspace` and
|
||||
`cargo test -p signed_state` (24 tests) both pass unchanged.
|
||||
2. ✅ **Remove the `tasks: Vec<Task<...>>` + `push_task` boilerplate**, in
|
||||
both `signed_state` (§6) and `crates/workspace` (§14), in favor of
|
||||
`.detach()`/`.detach_and_log_err(cx)`. Independent of every other change
|
||||
here, touches 10 files, all mechanical — and fixes a real unbounded-growth
|
||||
bug in `RepoDetailView`/`NewPullRequestView` along the way.
|
||||
|
||||
Done: both halves are complete, see §6 and §14 for details.
|
||||
3. **Fix the relay add/connect calls** (§8): drop `.as_str()`/`ToString`
|
||||
round trips, replace `add_relay` + blanket `client.connect()`/
|
||||
`connect_relay` pairs with `add_relay(url).and_connect()`, and delete
|
||||
|
||||
Reference in New Issue
Block a user