Skip to content

PythonExecutor has thread-safety risk due to process-level os.chdir() #103

@tanbro

Description

@tanbro

PythonExecutor has thread-safety risk due to process-level os.chdir()

Summary

The PythonExecutorCore class uses os.chdir() to change the working directory before executing Python code. Since os.chdir() modifies process-level state (affecting all threads in the process), this creates a thread-safety risk when multiple Python code executions run concurrently via asyncio.to_thread().

Affected Code

File: src/xagent/core/tools/core/python_executor.py

Location: PythonExecutorCore.execute_code() method

The code pattern (simplified):

old_cwd = None
if self.working_directory:
    old_cwd = os.getcwd()
    os.chdir(self.working_directory)

try:
    exec(code, safe_globals, local_vars)
    # ...
finally:
    if old_cwd is not None:
        os.chdir(old_cwd)

Called concurrently from: src/xagent/core/tools/adapters/vibe/python_executor.py

async def run_json_async(self, args: Mapping[str, Any]) -> Any:
    return await asyncio.to_thread(self.run_json_sync, args)

Problem Description

Root Cause

os.chdir() operates at the process level, not thread level. All threads in a Python process share the same current working directory (cwd). When multiple threads execute code that calls os.chdir(), they interfere with each other.

Race Condition Scenario

When two agents execute Python code concurrently:

Timeline:
─────────────────────────────────────────────────────────────────
Thread-A (Agent A)              Thread-B (Agent B)
─────────────────────────────────────────────────────────────────
os.chdir("/workspace/A/output")
                                os.chdir("/workspace/B/output")
                                ← Global cwd now points to B's directory
open("data.txt", "w")
← Actually opens /workspace/B/output/data.txt!
                                open("result.txt", "w")
os.chdir(old_cwd_A)             ← May restore to wrong directory
                                os.chdir(old_cwd_B)

Impact

  1. File I/O Confusion: Agent A's code may read/write Agent B's files
  2. Relative Path Breakage: open("config.json") may open unintended files
  3. Data Corruption: Concurrent writes to same filename may interleave
  4. Directory Restoration Failure: Exception before finally block may leave cwd in inconsistent state

Current Mitigation (Insufficient)

The code uses a try-finally block to restore the working directory:

finally:
    if old_cwd is not None:
        os.chdir(old_cwd)

However, this does not prevent race conditions because:

  • Other threads can execute os.chdir() during the execution window
  • The working directory is global process state, not thread-local
  • No synchronization mechanism protects the critical section

Additional Context

JavaScript Executor

The javascript_executor.py has a similar _get_working_directory() method that returns a working directory path. However, if the JavaScript executor spawns a subprocess (Node.js), it may not have the same race condition since subprocesses have independent cwd. This should be verified.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions