mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(fuse,cli): bound the stop's wait for busy threads (B165)
A stop joined every background thread for as long as each took. The sync engine's mirror pass calls Drive with no deadline, and a stop that waited for it was killed by systemd at TimeoutStopSec. The stop now waits up to 10 s for the background threads and FUSE workers together, names any still running, and the runtime shuts down within 2 s.
This commit is contained in:
1 parent
ce2bc0ac84
commit
573a29e596
5 files changed
+94
-12
No files matched your search
@@ -2090,6 +2090,9 @@ fn mount_once(mountpoint: Option<PathBuf>) -> Result<pdfs_fuse::MountOutcome> {
|
|||||||
if let Err(e) = rt.block_on(auth::persist(&session)) {
|
if let Err(e) = rt.block_on(auth::persist(&session)) {
|
||||||
tracing::debug!(error = %e, "no new tokens to persist on shutdown");
|
tracing::debug!(error = %e, "no new tokens to persist on shutdown");
|
||||||
}
|
}
|
||||||
|
// Dropping the runtime waits for every blocking task, however long; a call
|
||||||
|
// still stuck on Drive would hold the stop until systemd kills it (B165).
|
||||||
|
rt.shutdown_timeout(std::time::Duration::from_secs(2));
|
||||||
|
|
||||||
let outcome = result.context("mount failed")?;
|
let outcome = result.context("mount failed")?;
|
||||||
Ok(outcome)
|
Ok(outcome)
|
||||||
|
|||||||
@@ -784,9 +784,10 @@ fn signal_stop(rt: &tokio::runtime::Handle) -> std::sync::mpsc::Receiver<()> {
|
|||||||
sig_rx
|
sig_rx
|
||||||
}
|
}
|
||||||
|
|
||||||
/// How long teardown waits for the FUSE worker pool before proceeding without
|
/// How long teardown waits for the background workers and then the FUSE worker
|
||||||
/// it. Long enough for an in-flight block to finish, short enough that
|
/// pool before proceeding without them. Long enough for an in-flight block to
|
||||||
/// `systemctl --user stop` still returns inside `TimeoutStopSec`.
|
/// finish, short enough that `systemctl --user stop` still returns inside
|
||||||
|
/// `TimeoutStopSec`.
|
||||||
const WORKER_JOIN_DEADLINE: Duration = Duration::from_secs(10);
|
const WORKER_JOIN_DEADLINE: Duration = Duration::from_secs(10);
|
||||||
|
|
||||||
/// Signal every background loop, wake the ones that are blocked, and join them.
|
/// Signal every background loop, wake the ones that are blocked, and join them.
|
||||||
@@ -801,7 +802,8 @@ const WORKER_JOIN_DEADLINE: Duration = Duration::from_secs(10);
|
|||||||
/// inside `accept`, which only returns when a connection arrives — so it gets
|
/// inside `accept`, which only returns when a connection arrives — so it gets
|
||||||
/// one, from us, after the flag is already set. A join that takes a while is
|
/// one, from us, after the flag is already set. A join that takes a while is
|
||||||
/// expected and correct: a worker part-way through an upload finishes it rather
|
/// expected and correct: a worker part-way through an upload finishes it rather
|
||||||
/// than abandoning the user's bytes mid-flight.
|
/// than abandoning the user's bytes mid-flight. Up to [`WORKER_JOIN_DEADLINE`]:
|
||||||
|
/// past it, systemd kills the daemon anyway, and a queued op is sent again.
|
||||||
fn stop_workers(core: &Core, control_socket: &Path, workers: Vec<std::thread::JoinHandle<()>>) {
|
fn stop_workers(core: &Core, control_socket: &Path, workers: Vec<std::thread::JoinHandle<()>>) {
|
||||||
// Say so before the joins: a teardown that takes a while must not be read as
|
// Say so before the joins: a teardown that takes a while must not be read as
|
||||||
// a missed watchdog ping and turned into a kill.
|
// a missed watchdog ping and turned into a kill.
|
||||||
@@ -815,25 +817,71 @@ fn stop_workers(core: &Core, control_socket: &Path, workers: Vec<std::thread::Jo
|
|||||||
// unlink cannot put it back to sleep.
|
// unlink cannot put it back to sleep.
|
||||||
let _ = std::os::unix::net::UnixStream::connect(control_socket);
|
let _ = std::os::unix::net::UnixStream::connect(control_socket);
|
||||||
let _ = std::fs::remove_file(control_socket);
|
let _ = std::fs::remove_file(control_socket);
|
||||||
for worker in workers {
|
let deadline = Instant::now() + WORKER_JOIN_DEADLINE;
|
||||||
if worker.join().is_err() {
|
let running = join_until(workers, deadline);
|
||||||
warn!("a background worker panicked before shutdown");
|
if !running.is_empty() {
|
||||||
}
|
warn!(
|
||||||
|
running = running.join(","),
|
||||||
|
"background workers did not finish in time; shutting down around them"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
// The FUSE worker pool last, and *before* the caller drops the tokio
|
// The FUSE worker pool last, and *before* the caller drops the tokio
|
||||||
// runtime: a worker still inside a job when the runtime goes away panics on
|
// runtime: a worker still inside a job when the runtime goes away panics on
|
||||||
// its next timer with "A Tokio 1.x context was found, but it is being
|
// its next timer with "A Tokio 1.x context was found, but it is being
|
||||||
// shutdown" (seen 2026-09-18 09:58:35). Bounded, so a worker wedged on the
|
// shutdown" (seen 2026-09-18 09:58:35). Bounded, so a worker wedged on the
|
||||||
// network cannot hold the stop the user asked for.
|
// network cannot hold the stop the user asked for.
|
||||||
core.workers.stop_and_join(WORKER_JOIN_DEADLINE);
|
core.workers
|
||||||
|
.stop_and_join(deadline.saturating_duration_since(Instant::now()));
|
||||||
debug!("background workers stopped");
|
debug!("background workers stopped");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Join `workers` until `deadline`, and name the ones still running then.
|
||||||
|
///
|
||||||
|
/// A stop used to wait for each one however long it took. The sync engine's
|
||||||
|
/// mirror pass waits on Drive with no deadline, and a stop that waited for it
|
||||||
|
/// was killed by systemd (`docs/BUGS.md` B165).
|
||||||
|
fn join_until(workers: Vec<std::thread::JoinHandle<()>>, deadline: Instant) -> Vec<String> {
|
||||||
|
let mut running = Vec::new();
|
||||||
|
for worker in workers {
|
||||||
|
while !worker.is_finished() && Instant::now() < deadline {
|
||||||
|
std::thread::sleep(Duration::from_millis(10));
|
||||||
|
}
|
||||||
|
if !worker.is_finished() {
|
||||||
|
running.push(worker.thread().name().unwrap_or("unnamed").to_string());
|
||||||
|
} else if worker.join().is_err() {
|
||||||
|
warn!("a background worker panicked before shutdown");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
running
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use std::sync::atomic::{AtomicBool, Ordering};
|
use std::sync::atomic::{AtomicBool, Ordering};
|
||||||
|
use std::time::{Duration, Instant};
|
||||||
|
|
||||||
use super::{SecondaryInsertRejection, SecondaryMountRegistry, teardown_session};
|
use super::{SecondaryInsertRejection, SecondaryMountRegistry, join_until, teardown_session};
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_stop_names_the_workers_still_running_at_its_deadline() {
|
||||||
|
let (release, wait) = std::sync::mpsc::channel::<()>();
|
||||||
|
let done = std::thread::Builder::new()
|
||||||
|
.name("done".into())
|
||||||
|
.spawn(|| {})
|
||||||
|
.unwrap();
|
||||||
|
let stuck = std::thread::Builder::new()
|
||||||
|
.name("stuck".into())
|
||||||
|
.spawn(move || {
|
||||||
|
let _ = wait.recv();
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
let running = join_until(
|
||||||
|
vec![done, stuck],
|
||||||
|
Instant::now() + Duration::from_millis(100),
|
||||||
|
);
|
||||||
|
assert_eq!(running, ["stuck"]);
|
||||||
|
drop(release);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn session_is_unroutable_before_teardown_starts() {
|
fn session_is_unroutable_before_teardown_starts() {
|
||||||
|
|||||||
@@ -12,6 +12,34 @@ Conventions:
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## B165 — A stop waits on a stuck thread until systemd kills the daemon
|
||||||
|
|
||||||
|
**Status:** Fixed (unverified).
|
||||||
|
**Found:** 2026-10-04, by the account run on 2.10.0 (cleanup after on-demand pairing 2). The
|
||||||
|
daemon was stopped at 15:53:04 and logged "control socket closing", but never "sync engine
|
||||||
|
stopping" or "daemon stopping". systemd killed it 15 s later (`TimeoutStopSec`). The run's
|
||||||
|
cleanup then found no control socket, because the new daemon had not opened it yet, and left
|
||||||
|
its folders behind.
|
||||||
|
|
||||||
|
**Where:** `stop_workers` in `crates/pdfs-fuse/src/mount.rs`; `mount_once` in
|
||||||
|
`crates/pdfs-cli/src/main.rs`.
|
||||||
|
|
||||||
|
**Cause.** A stop joined every background thread and waited as long as each one took. Every
|
||||||
|
clean stop in the run logged the sync engine stopping just before the control socket
|
||||||
|
closed. This one did not, so the engine was busy. It was most likely in a mirror pass over the
|
||||||
|
run's mirror folder: that pass calls Drive with no deadline and does not check for a stop.
|
||||||
|
The thread is inferred from the log, not observed.
|
||||||
|
|
||||||
|
**Fix.** The stop waits up to 10 s for the background threads and the FUSE workers together,
|
||||||
|
then names any thread still running in a warning and exits. A queued op it cut off is sent
|
||||||
|
again on the next start, as after a kill. The runtime is shut down with a 2 s limit, so a
|
||||||
|
blocking task cannot hold the exit either. The pass itself still has no deadline (STATUS,
|
||||||
|
"Any time").
|
||||||
|
|
||||||
|
**Test:** `a_stop_names_the_workers_still_running_at_its_deadline` in `mount.rs`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## B164 — A folder lists a new file twice while its create lands
|
## B164 — A folder lists a new file twice while its create lands
|
||||||
|
|
||||||
**Status:** Fixed.
|
**Status:** Fixed.
|
||||||
|
|||||||
@@ -57,6 +57,8 @@ by its local id, in the queue, the stored tree and pins alike.
|
|||||||
before its contents are committed, and the mount took that answer (B163).
|
before its contents are committed, and the mount took that answer (B163).
|
||||||
- **A folder no longer lists a new file twice while it is uploaded.** A listing taken as the
|
- **A folder no longer lists a new file twice while it is uploaded.** A listing taken as the
|
||||||
upload landed showed it both as Drive had it and as the queue did (B164).
|
upload landed showed it both as Drive had it and as the queue did (B164).
|
||||||
|
- **Stopping the daemon no longer hangs until systemd kills it.** A stop waits up to 10 s for
|
||||||
|
work in progress, then exits and logs what was still running (B165).
|
||||||
- **A file renamed just as its upload finished keeps its new name.** The mount no longer showed
|
- **A file renamed just as its upload finished keeps its new name.** The mount no longer showed
|
||||||
the old name, and a second rename no longer failed with "No such file or directory" (B152).
|
the old name, and a second rename no longer failed with "No such file or directory" (B152).
|
||||||
- **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or
|
- **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
|||||||
- **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0).
|
- **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0).
|
||||||
- **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded
|
- **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded
|
||||||
locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c).
|
locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c).
|
||||||
- Fixes from the simulation and account runs since then: B127 and B141 to B164.
|
- Fixes from the simulation and account runs since then: B127 and B141 to B165.
|
||||||
- **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app,
|
- **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app,
|
||||||
tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`.
|
tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`.
|
||||||
|
|
||||||
@@ -100,7 +100,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
|||||||
into a deleted folder (B150, fixed). Not confirmed yet. The run on 2.10.0
|
into a deleted folder (B150, fixed). Not confirmed yet. The run on 2.10.0
|
||||||
(2026-10-04: 260 passed, 3 failed) failed a new file that read back empty (B163,
|
(2026-10-04: 260 passed, 3 failed) failed a new file that read back empty (B163,
|
||||||
fixed), a 128-entry folder that listed 129 (B164, fixed), and its cleanup, which ran
|
fixed), a 128-entry folder that listed 129 (B164, fixed), and its cleanup, which ran
|
||||||
while the daemon was restarting.
|
while the daemon was restarting: the stop waited on a busy thread until systemd killed
|
||||||
|
it (B165, bounded).
|
||||||
5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead.
|
5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead.
|
||||||
|
|
||||||
## After 3.0.0
|
## After 3.0.0
|
||||||
|
|||||||
Reference in new issue
Block a user