wraps(): concurrent or reentrant calls share and clear one statistics dict, violating thread-local contract
- 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