python / python/cpython

`test_profiling` sometimes times out under TSan

Open
#143,442 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

tests topic-profiling type-bug
Dominant language
Python
Stars
77.2k
Forks
35.9k
PR merge metrics
PR metrics pending

Description

We don't currently run test_profiling in the TSan CI, but the context is that I'm trying to get the entire test suite running under thread sanitizer (TSan).

Some of the tests in test_profiling fail to attach in time when run with thread sanitizer, because the sanitizer makes everything slower. This is especially the case when running tests in parallel with -j.

Here's a example:

ERROR: test_run_failed_script_live (test.test_profiling.test_sampling_profiler.test_live_collector_ui.TestLiveModeErrors.test_run_failed_script_live)
Test that running a failing script exits with clean error.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sgross/tsan-cpython/Lib/profiling/sampling/cli.py", line 279, in _run_with_sync
    _wait_for_ready_signal(sync_sock, process, _SYNC_TIMEOUT_SEC)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/sgross/tsan-cpython/Lib/profiling/sampling/cli.py", line 223, in _wait_for_ready_signal
    raise socket.timeout("timed out")
TimeoutError: timed out

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sgross/tsan-cpython/Lib/profiling/sampling/cli.py", line 1110, in _handle_live_run
    process = _run_with_sync(cmd, suppress_output=True)
  File "/home/sgross/tsan-cpython/Lib/profiling/sampling/cli.py", line 289, in _run_with_sync
    raise RuntimeError(
        "Process failed to signal readiness within timeout"
    )
RuntimeError: Process failed to signal readiness within timeout

I think the problem is that the the _SYNC_TIMEOUT_SEC of 5 seconds isn't enough in this case:

https://github.com/python/cpython/blob/4f9a8d075ee52b8f56aca14102c8bf9fd6f3c512/Lib/profiling/sampling/cli.py#L78

Here are some options to consider:

  1. Make _SYNC_TIMEOUT_SEC configurable via a command line option and specify a larger value for the tests (i.e., SHORT_TIMEOUT)
  2. Make _SYNC_TIMEOUT_SEC configurable via environment variable
  3. Skip these tests under TSan
  4. ???

cc @pablogsal

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in Lib/profiling/sampling/cli.py at _SYNC_TIMEOUT_SEC and _wait_for_ready_signal, then inspect the failing test_profiling case test_run_failed_script_live. Run the profiling tests under TSan, including parallel execution with -j, and compare existing test conventions for sanitizer-specific behavior. Done means the affected tests reliably complete under TSan without weakening normal test coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
performance, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.