jd / jd/tenacity

wraps(): concurrent or reentrant calls share and clear one statistics dict, violating thread-local contract

Open
#701 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
8.8k
Forks
359
Avg merge
1m
Merged PRs (30d)
1

Description

## Summary

`BaseRetrying.wraps()` binds the **same** `wrapped_f.statistics` dict object into every per-call retry copy (`copy._local.statistics = stats`) after clearing it. When the decorated function is invoked from two threads concurrently — or re-enters itself within one call stack — both invocations share, clear, and mutate one unlocked dict. Statistics from one call are destroyed mid-run by another, contradicting the documented contract that statistics are local to each call/thread.

Static analysis of `tenacity/__init__.py` on current master; not executed by the reporter.

## Minimal Reproduction (static)

```python
import threading
from tenacity import retry, stop_after_attempt

@retry(stop=stop_after_attempt(3))
def flaky():
... # fails a couple of times

threads = [threading.Thread(target=flaky) for _ in range(2)]
[t.start() for t in threads]
[t.join() for t in threads]

print(flaky.statistics)
# Expected: stats reflecting this invocation's own run
# Actual: whichever thread cleared/mutated `stats` last wins;
# attempt_number / start_time / idle_for are cross-contaminated
```

## Location

`tenacity/__init__.py`, `BaseRetrying.wraps` → inner `wrapped_f`:

```python
copy = self.copy()
# Reuse the same statistics dict rather than rebinding the attribute
# so that the stats stay visible through additional decorators that
# copy attributes via functools.wraps (which copies the reference to
# this dict into the outer wrapper's __dict__). See issue #519.
stats = wrapped_f.statistics # type: ignore[attr-defined]
stats.clear()
copy._local.statistics = stats # noqa: SLF001
self._local.statistics = stats
return copy(f, *args, **kw)
```

## Why This Is a Bug

1. The docstring/attribute contract states statistics values "are local to the thread running call" (see the comment block on `RetryCallState` / `statistics` ~25 lines above). Sharing and clearing a single dict across concurrent invocations breaks that guarantee.
2. The `#519` fix solved real visibility problem (outer decorators seeing stats through `functools.wraps`), but the implementation makes the *wrapper-level* dict a global rendezvous point: every concurrent entry point calls `stats.clear()` while another invocation is mid-retry-loop, so:
- `attempt_number` can reset to 0 between attempts,
- `start_time` reflects the wrong invocation,
- `idle_for`/`delay_since_first_attempt` accumulate across unrelated runs.
3. Recursive/re-entrant use (`f` calling itself through the same decorated wrapper) hits the identical clobbering single-threaded: the outer run's statistics are wiped when the inner call returns.

The pre-#519 behavior (fresh state via `self.copy()` without rebinding) was race-free but broke decorator-chaining visibility; the current code fixes visibility at the cost of isolation. Both properties should be satisfiable simultaneously.

## Suggested Direction

Keep publishing the final `RetryCallState` into `wrapped_f.statistics` *after* the invocation completes (preserving #519 visibility), but let each execution build its own state inside `copy._local` during the run:

```python
copy = self.copy()
try:
return copy(f, *args, **kw)
finally:
wrapped_f.statistics.clear()
wrapped_f.statistics.update(copy.statistics)
```

This preserves single-final-value semantics for readers while eliminating mid-run cross-thread mutation. A test spawning two threads against a multi-attempt decorated function and asserting each sees its own `attempt_number` progression would lock the behavior in.

## Environment

- tenacity master (post-#658), file `tenacity/__init__.py`
- Python 3.x (threading from stdlib)

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in tenacity/__init__.py at BaseRetrying.wraps and the RetryCallState/statistics contract comments. Trace how wrapped_f, copy._local, and copy.statistics behave during concurrent and reentrant calls, then add regression coverage for independent per-call attempt progression and final statistics visibility.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.