Skip to content

[FIX] DockerCommandLineCodeExecutor multi event loop aware #6402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

SongChiYoung
Copy link
Contributor

Why are these changes needed?

Problem

Previously, in DockerCommandLineCodeExecutor, cancellation tasks were added directly to self._cancellation_tasks using asyncio.create_task():

self._cancellation_tasks.append(asyncio.create_task(self._kill_running_command(command)))

This caused issues when cancellation tasks were created from multiple event loops, leading to loop mismatch errors during executor shutdown.

Solution
This PR fixes the issue by introducing a dedicated internal event loop for managing cancellation tasks.
Cancellation tasks are now scheduled in a fixed event loop using asyncio.run_coroutine_threadsafe():

                    future: ConcurrentFuture[None] = asyncio.run_coroutine_threadsafe(
                        self._kill_running_command(command), self._loop
                    )
                    self._cancellation_futures.append(future)

Additional Changes

  • Added detailed logging for easier debugging.
  • Ensured clean shutdown of the internal event loop and associated thread.

Note
This change ensures that all cancellation tasks are handled consistently in a single loop, preventing cross-loop conflicts and improving executor stability in multi-threaded environments.

Related issue number

Closes #6395

Checks

Copy link

codecov bot commented Apr 28, 2025

Codecov Report

Attention: Patch coverage is 71.73913% with 13 lines in your changes missing coverage. Please review.

Project coverage is 78.39%. Comparing base (63c791d) to head (daf1ff0).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ext/code_executors/docker/_docker_code_executor.py 71.73% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6402      +/-   ##
==========================================
- Coverage   78.42%   78.39%   -0.03%     
==========================================
  Files         221      221              
  Lines       16062    16102      +40     
==========================================
+ Hits        12596    12623      +27     
- Misses       3466     3479      +13     
Flag Coverage Δ
unittests 78.39% <71.73%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ekzhu ekzhu merged commit 516a211 into microsoft:main Apr 28, 2025
61 of 62 checks passed
abhinav-aegis pushed a commit to abhinav-aegis/autogen that referenced this pull request Apr 28, 2025
…#6402)

## Why are these changes needed?
*Problem*

Previously, in `DockerCommandLineCodeExecutor`, cancellation tasks were
added directly to `self._cancellation_tasks` using
`asyncio.create_task()`:
```python
self._cancellation_tasks.append(asyncio.create_task(self._kill_running_command(command)))
```
This caused issues when cancellation tasks were created from multiple
event loops, leading to loop mismatch errors during executor shutdown.

*Solution*
This PR fixes the issue by introducing a dedicated internal event loop
for managing cancellation tasks.
Cancellation tasks are now scheduled in a fixed event loop using
`asyncio.run_coroutine_threadsafe()`:
```python
                    future: ConcurrentFuture[None] = asyncio.run_coroutine_threadsafe(
                        self._kill_running_command(command), self._loop
                    )
                    self._cancellation_futures.append(future)
```

*Additional Changes*
- Added detailed logging for easier debugging.
- Ensured clean shutdown of the internal event loop and associated
thread.


*Note*
This change ensures that all cancellation tasks are handled consistently
in a single loop, preventing cross-loop conflicts and improving executor
stability in multi-threaded environments.

## Related issue number

<!-- For example: "Closes microsoft#1234" -->
Closes microsoft#6395 

## Checks

- [ ] I've included any doc changes needed for
<https://microsoft.github.io/autogen/>. See
<https://github.com/microsoft/autogen/blob/main/CONTRIBUTING.md> to
build and test documentation locally.
- [x] I've added tests (if relevant) corresponding to the changes
introduced in this PR.
- [x] I've made sure all auto checks have passed.

---------

Co-authored-by: Eric Zhu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DockerCommandLineCodeExecutor does not safely manage cancellation tasks across multiple event loops (loop mismatch issue)
2 participants