[Fix] Clean up shutdown routine for ClusterExecutors

[jan-janssen]

Summary by CodeRabbit

• Bug Fixes
  • Improved scheduler shutdown so cleanup is consistent across scenarios: in-flight work can now be reliably canceled or awaited and background threads/queues are always joined to prevent hangs and resource leaks.
• Tests
  • Added tests covering multiple shutdown behaviors (with/without waiting and with/without canceling in-flight work) and timing-based cases to validate shutdown robustness.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Centralizes shutdown branching in execute_tasks_h5 by introducing internal helpers _shutdown_executor and _cancel_futures, then routes memory refresh and process-cancellation flows through them; public APIs remain unchanged and thread/future join behavior in base shutdown is simplified.

Changes

Cohort / File(s)	Summary
Shutdown helpers & flow src/executorlib/task_scheduler/file/shared.py	Extracts shutdown logic into _shutdown_executor(wait, cancel_futures, ...), adds _cancel_futures(future_dict), and funnels existing _refresh_memory_dict / _cancel_processes calls through the centralized shutdown path; preserves task_done()/join() semantics.
Executor base shutdown tweak src/executorlib/task_scheduler/base.py	Removes wait conditional when a single Thread process is present so the thread and future queue are always joined; other cleanup unchanged.
Tests: shutdown behaviour tests/unit/executor/test_api.py	Adds add_with_sleep helper and multiple tests exercising shutdown combinations (wait vs no-wait, cancel futures true/false) and a direct test of _shutdown_executor; expands teardown to clean test directories.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Executor
    participant Shutdown as _shutdown_executor
    participant Futures as FutureDict
    participant Procs as Processes
    participant Mem as MemoryDict
    participant Q as Queue

    Client->>Shutdown: shutdown(wait?, cancel_futures?)
    alt cancel_futures == true
        Shutdown->>Futures: _cancel_futures(future_dict)
    end
    alt process cancellation required
        Shutdown->>Procs: _cancel_processes()
    end
    alt wait == true
        loop refresh until empty
            Shutdown->>Mem: _refresh_memory_dict()
        end
    else wait == false
        Shutdown->>Mem: _refresh_memory_dict()
    end
    Shutdown->>Q: task_done()
    Shutdown->>Q: join()
    Shutdown-->>Client: done

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• execute_tasks_h5() wait for future to complete #712: Also adjusts waiting-for-futures behavior in execute_tasks_h5; overlaps with the introduced wait/cancel logic.
• Upgrade terminate function #715: Modifies shutdown/termination handling in the same shared.py area touched by this change.
• Fix: Implement one _refresh_memory_dict() function #902: Works with _refresh_memory_dict / process-cancellation helpers that this refactor routes through.

Suggested reviewers

• liamhuber

Poem

🐇 I hopped through queues and futures bright,

I folded shutdown into one neat light.
Cancel, wait, or gently sweep—
task_done then join, now dreams to keep. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: refactoring and improving the shutdown routine for ClusterExecutors through code cleanup and reorganization.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch clusterexecutorshutdown

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.83%. Comparing base (d450829) to head (666ec2b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #920      +/-   ##
==========================================
+ Coverage   93.79%   93.83%   +0.03%     
==========================================
  Files          38       38              
  Lines        1951     1962      +11     
==========================================
+ Hits         1830     1841      +11     
  Misses        121      121              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.