Skip to content

Commit

Permalink
chore(tracing): remove atexit dead code and forksafe dead code (DataD…
Browse files Browse the repository at this point in the history
…og#8573)

In python 3.7 (what we support currently on main) `hasattr(atexit,
"unregister")` is always true, so we don't need to check it.
`hasattr(os, "register_at_fork")` is also ways true, so we don't need to
check it.

## Checklist

- [x] Change(s) are motivated and described in the PR description
- [x] Testing strategy is described if automated tests are not included
in the PR
- [x] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.
- [x] If change touches code that signs or publishes builds or packages,
or handles credentials of any kind, I've requested a review from
`@DataDog/security-design-and-guidance`.

## Reviewer Checklist

- [ ] Title is accurate
- [ ] All changes are related to the pull request's stated goal
- [ ] Description motivates each change
- [ ] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [ ] Testing strategy adequately addresses listed risks
- [ ] Change is maintainable (easy to change, telemetry, documentation)
- [ ] Release note makes sense to a user of the library
- [ ] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [ ] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
  • Loading branch information
ZStriker19 authored Mar 4, 2024
1 parent 688074a commit 6592d5d
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 69 deletions.
46 changes: 2 additions & 44 deletions ddtrace/internal/atexit.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,50 +16,8 @@
log = logging.getLogger(__name__)


if hasattr(atexit, "unregister"):
register = atexit.register
unregister = atexit.unregister
else:
# Hello Python 2!
_registry = [] # type: typing.List[typing.Tuple[typing.Callable[..., None], typing.Tuple, typing.Dict]]

def _ddtrace_atexit():
# type: (...) -> None
"""Wrapper function that calls all registered function on normal program termination"""
global _registry

# DEV: we make a copy of the registry to prevent hook execution from
# introducing new hooks, potentially causing an infinite loop.
for hook, args, kwargs in list(_registry):
try:
hook(*args, **kwargs)
except Exception:
# Mimic the behaviour of Python's atexit hooks.
log.exception("Error in atexit hook %r", hook)

def register(
func, # type: typing.Callable[..., typing.Any]
*args, # type: typing.Any
**kwargs, # type: typing.Any
):
# type: (...) -> typing.Callable[..., typing.Any]
"""Register a function to be executed upon normal program termination"""

_registry.append((func, args, kwargs))
return func

def unregister(func):
# type: (typing.Callable[..., typing.Any]) -> None
"""
Unregister an exit function which was previously registered using
atexit.register.
If the function was not registered, it is ignored.
"""
global _registry

_registry = [(f, args, kwargs) for f, args, kwargs in _registry if f != func]

atexit.register(_ddtrace_atexit)
register = atexit.register
unregister = atexit.unregister


# registers a function to be called when an exit signal (TERM or INT) or received.
Expand Down
26 changes: 1 addition & 25 deletions ddtrace/internal/forksafe.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,31 +69,7 @@ def unregister(after_in_child):
log.info("after_in_child hook %s was unregistered without first being registered", after_in_child.__name__)


if hasattr(os, "register_at_fork"):
os.register_at_fork(after_in_child=ddtrace_after_in_child, after_in_parent=set_forked)
elif hasattr(os, "fork"):
# DEV: This "should" be the correct way of implementing this, but it doesn't
# work if hooks create new threads.
_threading_after_fork = threading._after_fork # type: ignore

def _after_fork():
# type: () -> None
_threading_after_fork()
if not _soft:
ddtrace_after_in_child()

threading._after_fork = _after_fork # type: ignore[attr-defined]

# DEV: If hooks create threads, we should do this instead.
_os_fork = os.fork

def _fork():
pid = _os_fork()
if pid == 0 and _soft:
ddtrace_after_in_child()
return pid

os.fork = _fork
os.register_at_fork(after_in_child=ddtrace_after_in_child, after_in_parent=set_forked)

_resetable_objects = weakref.WeakSet() # type: weakref.WeakSet[ResetObject]

Expand Down

0 comments on commit 6592d5d

Please sign in to comment.