Avoid psutil lookup race when starting supervised task subprocesses

[DaveT1991]

Summary

Avoid a startup race in the task supervisor when starting forked task subprocesses.

WatchedSubprocess.start() currently wraps the freshly forked child with psutil.Process(pid) immediately. On some systems this can raise psutil.NoSuchProcess even though the child is the direct supervised process, which aborts task startup before the task instance is marked running and later shows up as a queued/failed mismatch.

This change uses a lightweight handle backed by os.waitpid/os.kill for directly forked children instead of performing an immediate psutil lookup, and adds a regression test to cover that startup path.

Closes #64974

Testing

• Added regression coverage in task-sdk/tests/task_sdk/execution_time/test_supervisor.py
• git diff --check

I could not run the Python test suite locally in this environment because python is not installed here.

[potiuk]

Nice. @kaxil @ashb @amoghrajesh -> that looks indeed plausible and the solution looks good as well. WDYT? That would explain a flurry of random task instance status not matching actually running tasks - there were quite a few reports about that one.

[ashb]

Mmmm I don't like the look of this -- The other advantage of psutil has is pid cycle/reuse detection.

Let me examine the issue too

[ashb]

I am not convinced this is the error -- Given psutil reads from the /proc fs filesystem on linux, and that comes directly from Kernel memory my understanding is that this cannot be racey. Once the process pid exists the /proc fs is updated and available.

I'm going to need more evidence that this is actually a race. We used psutil because it's safer than just killing a pid.

[potiuk]

I'm going to need more evidence that this is actually a race. We used psutil because it's safer than just killing a pid.

Yes. I saw similar issues raised and similar races happening at scalenin reported - by a quick search, we likely need to look closer and look at similar issues being solved elsewhere.

But the issue is real - we have seen at least few reports from our users that in Airflow 3 there are running tasks that are reported as failed - but they are actually running in background - which would definitely match the pattern described here.

[potiuk]

And yes - I agree that solution likely could be better if we find that this kind of race is really the root cause.

[ashb]

https://github.com/giampaolo/psutil/blob/v7.1.0/psutil/_pslinux.py#L1645-L1651:

            self._raise_if_zombie()
            # /proc/PID directory may still exist, but the files within
            # it may not, indicating the process is gone, see:
            # https://github.com/giampaolo/psutil/issues/2418
            if not os.path.exists(f"{self._procfs_path}/{pid}/stat"):
                raise NoSuchProcess(pid, name) from err
            raise

I think this is where the exception is coming from.

I wonder if this could be caused by something kube/containerd and a procfs proxy/causing a delay. I couldn't find any other reports about this

If that is the case I think I'd rather we do something like

try:
  proc = psutil.Process(pid)
except psutil.ProcessNotFound:
  time.sleep(0.1)
  proc = psutil.Process(pid)

However I'm still skeptical this is actually what's happening. I can find no other reports anywhere saying a pid can exist without /proc/pid/stat existing. If this kind of race was possible we can't be the first to hit it.

[ashb]

What might be worth doing is catching the error, and then when it does read the data from stdout/stderr sockets -- it's possible something is written on one those.

However even then, if the process has died it's procfs entry should stay around until something wait()s it!

[potiuk]

However I'm still skeptical this is actually what's happening. I can find no other reports anywhere saying a pid can exist without /proc/pid/stat existing. If this kind of race was possible we can't be the first to hit it.

I am also looking for those.

[DaveT1991]

Thanks for the feedback @ashb. I've updated the approach based on your suggestion — instead of replacing psutil, the new version simply retries psutil.Process(pid) after a 100ms sleep when NoSuchProcess is raised, which preserves pid-reuse detection.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[Copilot]

The monkeypatch targets here use a dotted-string that looks like a nested attribute path ("airflow.sdk.execution_time.supervisor.psutil.Process" / ".time.sleep"). With pytest's monkeypatch, the string form imports everything up to the last dot as a module, so this will try to import airflow.sdk.execution_time.supervisor.psutil and airflow.sdk.execution_time.supervisor.time (which don’t exist) and the test will error. Patch the attributes on the already-imported supervisor module objects instead (e.g. import airflow.sdk.execution_time.supervisor and set supervisor.psutil.Process / supervisor.time.sleep).

[Copilot]

The PR description says this change replaces the immediate psutil lookup with an os.waitpid/os.kill-backed lightweight handle, but the implementation here still wraps the child PID in psutil.Process (with a retry). Either update the PR description to match the actual approach, or implement the described non-psutil handle so reviewers/users aren’t misled about the behavior and dependencies.

[Copilot]

time.sleep(0.1) is a new hard-coded retry delay. Given this module already centralizes timing knobs as constants (e.g. MIN_HEARTBEAT_INTERVAL, SOCKET_CLEANUP_TIMEOUT), consider extracting this delay (and possibly retry count/timeout) into a named constant or config-backed value so it’s easier to tune and reason about if the race needs longer retries on some platforms.

[kaxil]

If you resolve comments from copilot, either add a comment if it is invalid or create a commit fixing it

[ashb]

Note however: I am still very very skeptical this is the fix