Skip to content

Fix spurious fd closing (os_process and v4l2)#13198

Open
hoshinolina wants to merge 2 commits intoobsproject:masterfrom
hoshinolina:bad-fd-closing
Open

Fix spurious fd closing (os_process and v4l2)#13198
hoshinolina wants to merge 2 commits intoobsproject:masterfrom
hoshinolina:bad-fd-closing

Conversation

@hoshinolina
Copy link
Contributor

@hoshinolina hoshinolina commented Mar 7, 2026

Description

Fix two bugs causing the wrong file descriptors to be closed. The os_process one is harmless but was discovered while debugging this. The v4l2loopback one is actively causing crashes.

Motivation and Context

User bug report of CEF crashes on profile switching or when closing OBS. Turns out CEF/Chromium validates fd reuse and actively asserts when something wonky happens.

How Has This Been Tested?

Tested locally, planning to push bugfix to Fedora shortly.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

The sequence should be the same for every fd:

- Close the parent end of the pipe
- If the child end of the pipe is not the intended fd already,
  - Dup it over
  - And close the old fd

This fixes a double-close() in the read path (which is fairly harmless
since it happens in the child, but sticks out in strace), some fd
leakage, and the stderr-already-is-2 case.
Make sure to use the invalid fd -1 when the output is not opened, and
only close valid fds. If fd 0 is closed, then this closes stdin. The
second time this happens, some other important fd will have become fd 0,
breaking something.

This causes random things to break (browser/CEF in reports, but really
it could be anything) as the wrong fds get closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants