fix(rcm): fix stuck child processes when a gevent application creates forks

[avara1986]

Description

Workaround (II) for remote config to work properly with gevent. We found that Remote Config create stuck/zombie child processes if an application with gevent uses os.fork.

What we do?

• Remove RemoteConfig.worker, because worker has PeriodicService as parent and PeriodicService use a worker too.
• Remove duplicated locks.
• Add methods like _stop_service, stop... using the same structure as TelemetryWriter and SpanWriter
• Remove enable() behavior. One of the previous fix (chore(iast): remote config thread workaround for gevent #4749) ensures that the Remote config worker thread is launched at the very end, not interrupting the gevent patching mechanism. With forksafe.register(cls._restart) we were calling twice to remoteconfig.enable(): one at fork time (and it raises the same error of the previous fix) and once again at register the products in the child process (this call is ok)

Caveat: this fix requires with env variable DD_GEVENT_PATCH_ALL=true but that's documented to be required on the gevent integration docs (we also added a reference to possible stuck processes if not set to ease the googleability of it).

How to reproduce the error

• Create a flask application:

import os
import time

from flask import Flask

app = Flask(__name__)

def start():
    pid1 = os.fork()
    if pid1 == 0:
        os.setsid()
        x = 2
        while x > 0:
            time.sleep(0.2)
            print("child pid = ", pid1)
            x -= 1
    else:
        os.waitpid(pid1, 0)

@app.route('/')
def index():
    start()
    return "OK"

• start an agent
• add this environment variables:

DD_GEVENT_PATCH_ALL=true
DD_REMOTE_CONFIGURATION_ENABLED=true

• run the application with

 ddtrace-run gunicorn -w 3 -k gevent app:app

• Add traffic. e.g:

#!/bin/bash
while :
do
    curl -s -k 'GET'  http://127.0.0.1:8000/
done

• Check your processes, you should have 4-5 gunicorn workers, if you see more than this number, you have dead processes. The could remain even after stopping gunicorn.

ps -aux | grep gunicorn
alberto+  132031  1.4  0.1 115840 63280 pts/0    S+   16:33   0:00 /py-39/bin/gunicorn -c gunicorn_conf.py app:app
alberto+  132070  0.4  0.2 201700 65172 pts/0    Sl+  16:33   0:00 /py-39/bin/gunicorn -c gunicorn_conf.py app:app
alberto+  132071  0.3  0.2 201716 65192 pts/0    Sl+  16:33   0:00 /py-39/bin/gunicorn -c gunicorn_conf.py app:app
alberto+  132072  0.3  0.2 201712 65184 pts/0    Sl+  16:33   0:00 /py-39/bin/gunicorn -c gunicorn_conf.py app:app

Another way to check is run apachebenchmark (ab) like:

ab  -n 1000000 -c 100 http://127.0.0.1:8001/

And running the script in another console. Eventually, the Flask app will stop responding (usually with 7 processes running on my machine).

You could check the gevent infinite lock with:

gdb -p 132072

or

py-spy record --native  --rate 300 -o my-flamegraph-result.svg --pid 132072

Checklist

• Change(s) are motivated and described in the PR description.
• Testing strategy is described if automated tests are not included in the PR.
• Risk is outlined (performance impact, potential for breakage, maintainability, etc).
• Change is maintainable (easy to change, telemetry, documentation).
• Library release note guidelines are followed.
• Documentation is included (in-code, generated user docs, public corp docs).
• Author is aware of the performance implications of this PR as reported in the benchmarks PR comment.

Reviewer Checklist

• Title is accurate.
• No unnecessary changes are introduced.
• Description motivates each change.
• Avoids breaking API changes unless absolutely necessary.
• Testing strategy adequately addresses listed risk(s).
• Change is maintainable (easy to change, telemetry, documentation).
• Release note makes sense to a user of the library.
• Reviewer is aware of, and discussed the performance implications of this PR as reported in the benchmarks PR comment.

[pr-commenter]

Benchmarks

Comparing candidate commit f72ae72 in PR branch avara1986/APPSEC-8131-rcm-forks-error with baseline commit 4372d46 in branch 1.x.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 56 cases.

[emmettbutler]

I pulled out the regression test from this change and ran it against 1.x (you can see the diff in #5130), and I found that the test hangs in CI. That tells us that the test replicates a real issue.