Skip to content

Commit e9d427f

Browse files
authored
Fix potential deadlock during process fork (#817)
* Fix a deadlock close_socket acquires _socket_lock, and calling it with the lock held deadlocks. On top of this the warning did not prove very useful, as it would also trigger when the application is sending metrics from another thread when the process forks, in which case the warning is a bit misleading. Without the warning there is no reason to check for socket being open, so just silently call close_socket() after the fork to avoid sharing the file descriptor between processes if another thread re-opened it between pre_fork() and fork itself. * Add test case for post_fork
1 parent 4dcf1ed commit e9d427f

File tree

2 files changed

+14
-4
lines changed

2 files changed

+14
-4
lines changed

datadog/dogstatsd/base.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1370,10 +1370,7 @@ def post_fork(self):
13701370

13711371
log.debug("[%d] post_fork for %s", os.getpid(), self)
13721372

1373-
with self._socket_lock:
1374-
if self.socket or self.telemetry_socket:
1375-
log.warning("Open socket detected after fork. Call pre_fork() before os.fork().")
1376-
self.close_socket()
1373+
self.close_socket()
13771374

13781375
self._forking = False
13791376

tests/unit/dogstatsd/test_statsd.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1963,3 +1963,16 @@ def test_max_payload_size(self):
19631963
self.assertEqual(statsd._max_payload_size, UDP_OPTIMAL_PAYLOAD_LENGTH)
19641964
statsd.socket_path = "/foo"
19651965
self.assertEqual(statsd._max_payload_size, UDS_OPTIMAL_PAYLOAD_LENGTH)
1966+
1967+
def test_post_fork_locks(self):
1968+
def inner():
1969+
statsd = DogStatsd(socket_path=None, port=8125)
1970+
# Statsd should survive this sequence of events
1971+
statsd.pre_fork()
1972+
statsd.get_socket()
1973+
statsd.post_fork()
1974+
t = Thread(target=inner)
1975+
t.daemon = True
1976+
t.start()
1977+
t.join(timeout=5)
1978+
self.assertFalse(t.is_alive())

0 commit comments

Comments
 (0)