[alexnorell]

What this fixes

capture_process_stderr redirects the process-wide fd 2 into a pipe around each
cv2.VideoCapture open, so FFmpeg's error text can be classified into a
StreamErrorCode. It did that without any synchronisation, and every
InferencePipeline opens its source on its own thread. When two sources start
together the redirects interleave:

A: pipeA; savedA = dup(2); dup2(wA, 2)
B: pipeB; savedB = dup(2)   <- a duplicate of wA, not of the real stderr
B: dup2(wB, 2)
A: dup2(savedA, 2); close(savedA); close(wA); read(rA)

A's read never sees EOF, because savedB is still a write end of pipeA, so A
blocks there forever. B then restores savedB, leaving fd 2 permanently pointing
at pipeA's write end: the process loses its real stderr, and once that orphaned
64 KiB pipe fills, any native stderr writer blocks too.

The observable symptom is that the source created first sits in
INITIALISING at 0 fps with no error until the process is restarted, which
repeats the race. Two RTSP sources started concurrently reproduce it every time.

What changed

Because fd 2 is process-wide, the redirection is now process-wide too: a single
reference-counted capture that the first entrant installs and the last leaver
removes. The state lock is held only for the fd manipulation itself and never
across an open, so a slow or unreachable camera cannot starve any other source's
connect or reconnect. fd 2 is restored under that lock — which is what stops the
next entrant from duplicating a pipe instead of the real stderr — and on the
exception path as well.

The pipe is drained continuously by a background thread instead of being read
once after the open. That also fixes a second hang, independent of the race:
anything writing more than the 64 KiB pipe capacity during a single open blocked
inside the open, because nothing was reading the pipe yet.

Each participant gets the text written during its own window, delimited without
writing anything into the pipe: the pipe is asked how much it still holds
(FIONREAD) and the drain thread reports when it is idle, under a short bounded
timeout, so a window is not cut short by output the drain thread has not copied
yet. Overlapping captures can still see each other's lines. That is harmless and
documented: the captured text is only a hint for classification and is consulted
solely when isOpened() is False, so another source's output can never turn a
successful open into an error.

Every wait is bounded. Reference release is unconditional, so an interruption
anywhere in the bookkeeping still restores fd 2, and startup cleans up on
BaseException — including an interrupt inside Thread.start() that may already
have spawned the drain. Descriptors are closed through a helper that gives up
ownership of the number before closing it, so no cleanup path can shut a file the
process has since been handed that number for. Startup duplicates the real stderr
before allocating the pipe, so a process whose stderr is closed degrades to no
capture rather than having os.pipe() handed descriptor 2.

A forked child gets particular care, since it inherits descriptors but none of
the threads. The at-fork handler drops the inherited redirection without touching
any per-instance lock — another thread may have held one when fork() ran — and
a context manager unwinding in the child leaves the inherited object alone. It
also closes the read end of any capture that had been released but whose drain
thread had not yet finished with it; that thread does not exist in the child, so
nothing else would.

Readiness uses poll() rather than select(), which raises ValueError once a
descriptor reaches FD_SETSIZE — reachable on a server holding many sources and
sockets, and fatal for a thread whose job is to drain a pipe. On Windows, which
has neither poll() nor pipe-capable select(), the drain thread falls back to
the blocking read / EOF-on-teardown mechanism the previous code used.

New DISABLE_NATIVE_STDERR_CAPTURE (default False, i.e. capture enabled) turns
the capture off entirely, at the cost of the underlying error text in
SourceConnectionError messages. capture_process_stderr.handles_concurrent_opens
and NATIVE_STDERR_CAPTURE_HANDLES_CONCURRENT_OPENS are exported so anything
that worked around the old behaviour by serialising native opens can detect a
build that no longer needs it and disable itself.

Deliberately unchanged

Two behaviours are left exactly as they were, and are now recorded in the
docstring. While a capture is active fd 2 belongs to it, so stderr written by the
rest of the process is captured rather than logged; and a subprocess spawned
during a capture inherits the pipe and sees EPIPE on stderr writes made after the
capture ends. Both predate this change. Neither can be addressed without work
that does not belong in a concurrency fix — in particular, mirroring captured
bytes back to the log naively would publish the credential-bearing source
references that some backend error messages contain, so doing it safely needs
line-boundary reassembly, redaction before forwarding, and a forwarding path that
cannot stall behind a slow log sink. I would rather do that separately.

Not included

cv2.VideoCapture RTSP opens are already bounded: OpenCV's FFmpeg wrapper
enforces a 30 s open timeout through its interrupt callback (measured — Stream timeout triggered after 30034.86 ms). Note that FFmpeg's stimeout in
OPENCV_FFMPEG_CAPTURE_OPTIONS does not shorten it, while
CAP_PROP_OPEN_TIMEOUT_MSEC passed as a capture parameter does (measured at
4.00 s). No timeout machinery was added.

Testing

New test_stream_error_classifier_concurrency.py: eighteen tests, each on a
daemon thread joined with a bound or isolated in a subprocess with a hard
timeout, so a regression fails the suite instead of hanging it. They cover the
exact interleaving above; more than one pipe capacity written during a single
capture; a read descriptor forced above FD_SETSIZE; interrupts injected into
the entry bookkeeping, the exit bookkeeping and Thread.start(); every cleanup
path run again over a spent capture whose descriptor numbers have been reused; a
process whose stderr is closed; a fork() taken while another thread holds the
capture's condition; a fork() taken while a released capture's drain is still
winding down; fd 2 still referring to the original stderr (st_dev/st_ino)
after N interleaved captures, including when the bodies raise; per-participant
windows; the opt-out; the capability marker; and the invariant that the module
never writes into the capture pipe at all.

Fourteen of the eighteen fail on main. Repeat runs show no flakiness, and 10k
sequential plus 10k concurrent capture cycles return descriptor and thread counts
to their baseline. The camera directory passes (292), and the full unit-test
invocation matches main exactly — the same four pre-existing, order-dependent
test_builder.py symlink failures on both. black, isort and flake8 are
clean over inference inference_sdk.

[github-actions]

👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely.

🚦 This PR is marked Ready for review, so automated Claude review will run — and every pass spends real credits.

Warning

💸 The Claude reviewer bills in credits, not vibes

Automated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared.

Draft when unsure, Ready when you mean it:

• 🌱 Not sure the PR is in good shape yet? Keep it (or set it back) as a draft — drafts pause review, so you can push and iterate without burning credits on a moving target.
• 💪 Feel strong about the contents? Mark it Ready for review and the reviewer will take a look.

However you get there, arrive prepared:

• 🧱 Bring a SOLID, thorough PR. Point your local agent at our skills/ to tune it to our guidelines first — or, if you are one of those fabled carbon-based contributors, read them yourself. A half-baked diff costs exactly the same to review as a finished one.
• ✅ Resolve every comment before you re-request review. Re-requesting with threads still open means paying twice for the same conversation.
• 🔁 Do not use CI review as an inner loop for a local agent. The reviewer is not a step-by-step debugger — do the unfolding locally and arrive with the answer, not the search.
• 🙋 If something looks off, ask a human. One question to a maintainer is cheaper and faster than three rounds of agent re-review chasing a misread.

Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.

• Prefer to skip automated review entirely? Add the skip-claude-review label.

[github-actions]

🤖 Claude review started at commit b7c82d9ee20909c0441f49df001afb6a0cf349f2.

New commits are not auto-reviewed. Add the claude-review label to request a re-review — the label is consumed when the review starts, so just add it again next time.

[github-actions]

Review summary

Skills: review-topic-concurrency-and-resource-safety, review-core-infra, review-topic-test-hygiene, review-topic-backward-compat-and-versioning.

No dedicated surface skill covers inference/core/interfaces/camera/** — generic review plus topic skills only.

Scope: reference-counted, lock-guarded rewrite of the process-wide fd-2 stderr capture around native cv2.VideoCapture opens, plus a new DISABLE_NATIVE_STDERR_CAPTURE opt-out and a concurrency regression test module.

Traced under zero-trust:

• fd manipulation is always under _capture_state_lock and never spans the open; ref_count is only mutated under that lock; the drain thread is the sole owner/closer of the read fd; restore_stderr is one-shot and gated by _active_capture is self, so a late or dead-drain restore cannot clobber a newer capture.
• every wait is bounded (quiesce deadline, poll timeout); start(), entry, exit, and fork paths all give the reference back / restore fd 2 / close descriptors (via the pop-then-close helper) on BaseException; the at-fork handler runs single-threaded and touches no per-instance lock.
• the caller (video_source.py:152) still consumes the yielded List[str] unchanged; no other consumers of the contract exist.
• tests are bounded (daemon-thread / subprocess with hard timeouts) and assert the real behaviors rather than mocking the changed path.

No blocking or medium+ findings. The new env var mirrors existing undocumented streaming flags; env-var docs live in the external roboflow/docs repo, and this surface is not a changelog-bound subsystem (inference_models / Execution Engine), so no changelog entry is required.

Reviewed at HEAD: b7c82d9

[github-actions]

😎 PR passes the vibe-check and trust-me-bro verification.