Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Walkthrough

The changes add script-path metadata to pickled cross-interpreter values and retry selected unpickling failures with an isolated script namespace. Threaded interpreter finalization now records the finalizing thread and uses its identifier in signal checks.

Changes

Script globals during cross-interpreter unpickling

Threaded interpreter finalization

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant pickle_dumps
  participant PickledData
  participant pickle_loads
  participant isolated_main
  participant runpy.run_path
  pickle_dumps->>PickledData: Store pickle bytes and optional script path
  PickledData->>pickle_loads: Provide bytes and saved path
  pickle_loads->>pickle_loads: Try ordinary pickle loading
  pickle_loads->>isolated_main: Request namespace after matching AttributeError
  isolated_main->>runpy.run_path: Execute saved path with a fake name
  runpy.run_path-->>isolated_main: Return script namespace
  isolated_main-->>pickle_loads: Return cached namespace for retry

Suggested reviewers: youknowone

Merge Risk: 🔵 Low · up to 889e0

Cross-interpreter calls from different scripts can fail or use the wrong script’s globals when they share a target interpreter. This is a bounded case that should be fixed or explicitly accepted before merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 889e0

A receiving interpreter can now run a recorded script while restoring an object. It also caches one script namespace for later calls, even when those calls originate from another script. The retry is narrow, but the trust of recorded paths and the effect of sharing an interpreter across scripts remain uncertain.

Retained concerns

• Medium · security · inferred: The target caches one isolated main namespace without associating it with the saved script path. If the same target receives payloads from different scripts, later missing-class retries can fail against the first script's namespace or resolve a colliding name to its class.
• Medium · security · inferred: A narrowly triggered reconstruction retry executes the producer's saved script path in the target before the custom class-lookup audit. The changed routine has no explicit path validation. Whether this crosses an effective authority boundary depends on path trust and target privileges that are not established here.

Security Blast Radius

• inferred — The new script execution and namespace cache operate in the selected target interpreter. The supplied evidence does not establish a separate tenant, service, or privilege boundary, or authority beyond the existing callable and pickle call path.

Security Findings and Attack Paths

• inferred — If producers with different script paths call the same target, the first successful fallback can determine which classes later main names resolve to. Distinct names can fail; colliding names can resolve to the first script's class.

Trust Boundaries and Controls

• observed — The code first attempts ordinary pickle loading and limits retry to a matching main missing-attribute error. It audits custom class lookup but shows no explicit validation of the saved path before run_path executes it.

Resilience and Maintainability Implications

• inferred — Locking serializes first cache initialization, and a failed run_path does not populate the cache. Script side effects may nevertheless occur before a failed retry returns the original unpickling error; a successful cache entry persists for subsequent retries.

Hardening Proposals

• proposed — Bind the cached namespace to the payload's script provenance, or reject a fallback whose path differs from the cached path.
• proposed — Define the trusted-path and target-authority policy for script replay, and enforce or audit that policy before run_path where required.

❌ Failed checks (1 warning)

• Create a new PR

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.

• X
• Mastodon
• Reddit
• LinkedIn

Comment @coderabbitai help to get the list of available commands.

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] test: cpython/Lib/test/test_pyrepl (TODO: 19)
[ ] test: cpython/Lib/test/test_repl.py (TODO: 5)

dependencies:

dependent tests: (no tests depend on pyrepl)

[ ] lib: cpython/Lib/concurrent
[ ] test: cpython/Lib/test/test_concurrent_futures (TODO: 1)
[ ] test: cpython/Lib/test/test_interpreters
[ ] test: cpython/Lib/test/test__interpreters.py
[ ] test: cpython/Lib/test/test__interpchannels.py
[ ] test: cpython/Lib/test/test_crossinterp.py

dependencies:

• concurrent (native: _crossinterp, _interpqueues, _interpreters, _queues, concurrent.futures, concurrent.futures._base, interpreter, itertools, multiprocessing.connection, multiprocessing.queues, multiprocessing.synchronize, process, sys, thread, time)
  • logging (native: atexit, collections.abc, email.message, email.utils, errno, http.client, logging.handlers, multiprocessing.queues, select, sys, time, urllib.parse, win32evtlog, win32evtlogutil)
  • multiprocessing (native: _multiprocessing, _posixshmem, _posixsubprocess, _winapi, array, atexit, collections.abc, connection, context, dummy, errno, forkserver, heap, itertools, managers, mmap, msvcrt, multiprocessing.connection, pool, popen_fork, popen_forkserver, popen_spawn_posix, popen_spawn_win32, queues, resource_sharer, resource_tracker, sharedctypes, spawn, synchronize, sys, time, util, xmlrpc.client)
  • pickle (native: _pickle, itertools, sys)
  • collections, functools, os, queue, threading, traceback, types, weakref

dependent tests: (17 tests)

• concurrent: test_asyncio test_compileall test_concurrent_futures test_context test_genericalias test_inspect test_struct test_sys test_threading test_types test_wmi
  • asyncio: test_asyncio test_external_inspection test_logging test_os test_pdb test_unittest

Legend:

• [+] path exists in CPython
• [x] up-to-date, [ ] outdated

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1

Comparing 1ndahous3:concurrent_futures_refresh (889e0a7) with main (a7d75d2)

Footnotes

1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩