fix(process): avoid leaking zombies and fds in detached processes - #3379

Merged
cgutman merged 1 commit into
LizardByte:masterfrom
cgutman:zombie_leaks
Nov 8, 2024
Merged

fix(process): avoid leaking zombies and fds in detached processes#3379
cgutman merged 1 commit into
LizardByte:masterfrom
cgutman:zombie_leaks

Conversation

@cgutman

@cgutmancgutman commented Nov 8, 2024•
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

We were leaking detached child processes on Mac and Linux because we are expected to still wait on those with waitpid() in order to reap their entry in the process table. We will now do that when proc_t::running() is called (which happens regularly while streaming and periodically when clients poll for server info). This isn't a perfect solution but it's much better than the current situation today where all detached processes become zombies.

We were also leaking file descriptors into detached processes which could cause our log files and sockets to persist longer than expected.

Screenshot

Issues Fixed or Closed

Fixes#3378

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

cgutman
cgutman commented Nov 8, 2024
View reviewed changes
Comment threadsrc/platform/linux/misc.cpp

bp::child
run_command(bool elevated, bool interactive, const std::string &cmd, boost::filesystem::path &working_dir, const bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) {
// clang-format off

@cgutmancgutmanNov 8, 2024•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format gets confused by the boost::process syntax and thinks bp::std_in is a template class bp::std_in<bp::null, bp::std_out>

@codecov

codecovBot commented Nov 8, 2024•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 11.05%. Comparing base (54ddf37) to head (627b64f).
Report is 86 commits behind head on master.

Files with missing linesPatch %Lines
src/platform/linux/misc.cpp0.00%4 Missing ⚠️
src/platform/macos/misc.mm0.00%4 Missing ⚠️
src/process.cpp0.00%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3379 +/- ##
==========================================
- Coverage 11.11% 11.05% -0.06% 
==========================================
Files 75 99 +24 Lines 13204 17237 +4033 Branches 6084 8036 +1952 ==========================================
+ Hits 1467 1905 +438 - Misses 9307 12783 +3476 - Partials 2430 2549 +119
FlagCoverage Δ
Linux8.34% <0.00%> (?)
Windows5.23% <ø> (ø)
macOS-1313.62% <0.00%> (-0.02%)⬇️
macOS-1412.62% <0.00%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing linesCoverage Δ
src/process.cpp2.68% <0.00%> (+0.53%)⬆️
src/platform/linux/misc.cpp9.23% <0.00%> (ø)
src/platform/macos/misc.mm8.74% <0.00%> (-0.04%)⬇️

... and 44 files with indirect coverage changes

@sonarqubecloud

sonarqubecloudBot commented Nov 8, 2024

Copy link
Copy Markdown

Quality Gate PassedQuality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@cgutman
cgutman merged commit 1cab1a5 into LizardByte:masterNov 8, 2024
qiin2333 referenced this pull request in AlkaidLab/foundation-sunshine Nov 13, 2024
@cgutman@qiin2333
fix(process): avoid leaking zombies and fds in detached processes (#3…
f6d1b51
…379)
This was referenced Jan 18, 2025
Merged
Closed
Merged
@ReenigneArcherReenigneArcher mentioned this pull request Jan 22, 2025
Merged
c2vi pushed a commit to c2vi/Sunshine that referenced this pull request Mar 27, 2025
@cgutman@c2vi
fix(process): avoid leaking zombies and fds in detached processes (Li…
3c14121
…zardByte#3379)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ReenigneArcherReenigneArcherReenigneArcher approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Detached Processes launched by sunshine end up as zombies on linux after closing them

2 participants

@cgutman@ReenigneArcher
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix(process): avoid leaking zombies and fds in detached processes - #3379
Merged
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Nov 8, 2024
Merged
fix(process): avoid leaking zombies and fds in detached processes#3379
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Conversation
@cgutman
@cgutmancgutman
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
We were leaking detached child processes on Mac and Linux because we are expected to still wait on those with waitpid() in order to reap their entry in the process table. We will now do that when proc_t::running() is called (which happens regularly while streaming and periodically when clients poll for server info). This isn't a perfect solution but it's much better than the current situation today where all detached processes become zombies.
We were also leaking file descriptors into detached processes which could cause our log files and sockets to persist longer than expected.
Screenshot
Issues Fixed or Closed
Fixes#3378
Type of Change
  •  Bug fix (non-breaking change which fixes an issue)
  •  New feature (non-breaking change which adds functionality)
  •  Breaking change (fix or feature that would cause existing functionality to not work as expected)
  •  Dependency update (updates to dependencies)
  •  Documentation update (changes to documentation)
  •  Repository update (changes to repository files, e.g. .github/...)
Checklist
  •  My code follows the style guidelines of this project
  •  I have performed a self-review of my own code
  •  I have commented my code, particularly in hard-to-understand areas
  •  I have added or updated the in code docstring/documentation-blocks for new or existing methods/components
cgutman
cgutman
commented
Nov 8, 2024
View reviewed changes
Comment threadsrc/platform/linux/misc.cpp

 bp::child
 run_command(bool elevated, bool interactive, const std::string &cmd, boost::filesystem::path &working_dir, const bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) {
 // clang-format off
@cgutmancgutmanNov 8, 2024•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format gets confused by the boost::process syntax and thinks bp::std_in is a template class bp::std_in<bp::null, bp::std_out>
@codecov
codecovBot
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
Project coverage is 11.05%. Comparing base (54ddf37) to head (627b64f).

Report is 86 commits behind head on master.
Files with missing linesPatch %Lines
src/platform/linux/misc.cpp0.00%4 Missing ⚠️
src/platform/macos/misc.mm0.00%4 Missing ⚠️
src/process.cpp0.00%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3379 +/- ##
==========================================
- Coverage 11.11% 11.05% -0.06% 
==========================================
Files 75 99 +24 Lines 13204 17237 +4033 Branches 6084 8036 +1952 ==========================================
+ Hits 1467 1905 +438 - Misses 9307 12783 +3476 - Partials 2430 2549 +119 
FlagCoverage Δ
Linux8.34% <0.00%> (?)
Windows5.23% <ø> (ø)
macOS-1313.62% <0.00%> (-0.02%)⬇️
macOS-1412.62% <0.00%> (-0.01%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Files with missing linesCoverage Δ
src/process.cpp2.68% <0.00%> (+0.53%)⬆️
src/platform/linux/misc.cpp9.23% <0.00%> (ø)
src/platform/macos/misc.mm8.74% <0.00%> (-0.04%)⬇️
... and 44 files with indirect coverage changes
@sonarqubecloud
sonarqubecloudBot
commented
Nov 8, 2024

Copy link


Copy Markdown

Quality Gate PassedQuality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
@cgutman
cgutman
merged commit 1cab1a5
into
LizardByte:masterNov 8, 2024
qiin2333 referenced
this pull request
in AlkaidLab/foundation-sunshine
Nov 13, 2024
@cgutman@qiin2333
fix(process): avoid leaking zombies and fds in detached processes (#3…
f6d1b51
…379)

This was referenced Jan 18, 2025
 Merged

 Closed

 Merged

@ReenigneArcherReenigneArcher
mentioned this pull request
Jan 22, 2025
 Merged

c2vi pushed a commit
to c2vi/Sunshine
that referenced
this pull request
Mar 27, 2025
@cgutman@c2vi
fix(process): avoid leaking zombies and fds in detached processes (Li…
3c14121
…zardByte#3379)
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ReenigneArcherReenigneArcherReenigneArcher approved these changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Detached Processes launched by sunshine end up as zombies on linux after closing them


2 participants

@cgutman@ReenigneArcher

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(process): avoid leaking zombies and fds in detached processes - #3379
Merged
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Nov 8, 2024
Merged
fix(process): avoid leaking zombies and fds in detached processes#3379
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Conversation
@cgutman
@cgutmancgutman
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
We were leaking detached child processes on Mac and Linux because we are expected to still wait on those with waitpid() in order to reap their entry in the process table. We will now do that when proc_t::running() is called (which happens regularly while streaming and periodically when clients poll for server info). This isn't a perfect solution but it's much better than the current situation today where all detached processes become zombies.
We were also leaking file descriptors into detached processes which could cause our log files and sockets to persist longer than expected.
Screenshot
Issues Fixed or Closed
Fixes#3378
Type of Change
  •  Bug fix (non-breaking change which fixes an issue)
  •  New feature (non-breaking change which adds functionality)
  •  Breaking change (fix or feature that would cause existing functionality to not work as expected)
  •  Dependency update (updates to dependencies)
  •  Documentation update (changes to documentation)
  •  Repository update (changes to repository files, e.g. .github/...)
Checklist
  •  My code follows the style guidelines of this project
  •  I have performed a self-review of my own code
  •  I have commented my code, particularly in hard-to-understand areas
  •  I have added or updated the in code docstring/documentation-blocks for new or existing methods/components
cgutman
cgutman
commented
Nov 8, 2024
View reviewed changes
Comment threadsrc/platform/linux/misc.cpp

 bp::child
 run_command(bool elevated, bool interactive, const std::string &cmd, boost::filesystem::path &working_dir, const bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) {
 // clang-format off
@cgutmancgutmanNov 8, 2024•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format gets confused by the boost::process syntax and thinks bp::std_in is a template class bp::std_in<bp::null, bp::std_out>
@codecov
codecovBot
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
Project coverage is 11.05%. Comparing base (54ddf37) to head (627b64f).

Report is 86 commits behind head on master.
Files with missing linesPatch %Lines
src/platform/linux/misc.cpp0.00%4 Missing ⚠️
src/platform/macos/misc.mm0.00%4 Missing ⚠️
src/process.cpp0.00%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3379 +/- ##
==========================================
- Coverage 11.11% 11.05% -0.06% 
==========================================
Files 75 99 +24 Lines 13204 17237 +4033 Branches 6084 8036 +1952 ==========================================
+ Hits 1467 1905 +438 - Misses 9307 12783 +3476 - Partials 2430 2549 +119 
FlagCoverage Δ
Linux8.34% <0.00%> (?)
Windows5.23% <ø> (ø)
macOS-1313.62% <0.00%> (-0.02%)⬇️
macOS-1412.62% <0.00%> (-0.01%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Files with missing linesCoverage Δ
src/process.cpp2.68% <0.00%> (+0.53%)⬆️
src/platform/linux/misc.cpp9.23% <0.00%> (ø)
src/platform/macos/misc.mm8.74% <0.00%> (-0.04%)⬇️
... and 44 files with indirect coverage changes
@sonarqubecloud
sonarqubecloudBot
commented
Nov 8, 2024

Copy link


Copy Markdown

Quality Gate PassedQuality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
@cgutman
cgutman
merged commit 1cab1a5
into
LizardByte:masterNov 8, 2024
qiin2333 referenced
this pull request
in AlkaidLab/foundation-sunshine
Nov 13, 2024
@cgutman@qiin2333
fix(process): avoid leaking zombies and fds in detached processes (#3…
f6d1b51
…379)

This was referenced Jan 18, 2025
 Merged

 Closed

 Merged

@ReenigneArcherReenigneArcher
mentioned this pull request
Jan 22, 2025
 Merged

c2vi pushed a commit
to c2vi/Sunshine
that referenced
this pull request
Mar 27, 2025
@cgutman@c2vi
fix(process): avoid leaking zombies and fds in detached processes (Li…
3c14121
…zardByte#3379)
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ReenigneArcherReenigneArcherReenigneArcher approved these changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Detached Processes launched by sunshine end up as zombies on linux after closing them


2 participants

@cgutman@ReenigneArcher

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(process): avoid leaking zombies and fds in detached processes - #3379
Merged
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Nov 8, 2024
Merged
fix(process): avoid leaking zombies and fds in detached processes#3379
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Conversation
@cgutman
@cgutmancgutman
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
We were leaking detached child processes on Mac and Linux because we are expected to still wait on those with waitpid() in order to reap their entry in the process table. We will now do that when proc_t::running() is called (which happens regularly while streaming and periodically when clients poll for server info). This isn't a perfect solution but it's much better than the current situation today where all detached processes become zombies.
We were also leaking file descriptors into detached processes which could cause our log files and sockets to persist longer than expected.
Screenshot
Issues Fixed or Closed
Fixes#3378
Type of Change
  •  Bug fix (non-breaking change which fixes an issue)
  •  New feature (non-breaking change which adds functionality)
  •  Breaking change (fix or feature that would cause existing functionality to not work as expected)
  •  Dependency update (updates to dependencies)
  •  Documentation update (changes to documentation)
  •  Repository update (changes to repository files, e.g. .github/...)
Checklist
  •  My code follows the style guidelines of this project
  •  I have performed a self-review of my own code
  •  I have commented my code, particularly in hard-to-understand areas
  •  I have added or updated the in code docstring/documentation-blocks for new or existing methods/components
cgutman
cgutman
commented
Nov 8, 2024
View reviewed changes
Comment threadsrc/platform/linux/misc.cpp

 bp::child
 run_command(bool elevated, bool interactive, const std::string &cmd, boost::filesystem::path &working_dir, const bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) {
 // clang-format off
@cgutmancgutmanNov 8, 2024•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format gets confused by the boost::process syntax and thinks bp::std_in is a template class bp::std_in<bp::null, bp::std_out>
@codecov
codecovBot
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
Project coverage is 11.05%. Comparing base (54ddf37) to head (627b64f).

Report is 86 commits behind head on master.
Files with missing linesPatch %Lines
src/platform/linux/misc.cpp0.00%4 Missing ⚠️
src/platform/macos/misc.mm0.00%4 Missing ⚠️
src/process.cpp0.00%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3379 +/- ##
==========================================
- Coverage 11.11% 11.05% -0.06% 
==========================================
Files 75 99 +24 Lines 13204 17237 +4033 Branches 6084 8036 +1952 ==========================================
+ Hits 1467 1905 +438 - Misses 9307 12783 +3476 - Partials 2430 2549 +119 
FlagCoverage Δ
Linux8.34% <0.00%> (?)
Windows5.23% <ø> (ø)
macOS-1313.62% <0.00%> (-0.02%)⬇️
macOS-1412.62% <0.00%> (-0.01%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Files with missing linesCoverage Δ
src/process.cpp2.68% <0.00%> (+0.53%)⬆️
src/platform/linux/misc.cpp9.23% <0.00%> (ø)
src/platform/macos/misc.mm8.74% <0.00%> (-0.04%)⬇️
... and 44 files with indirect coverage changes
@sonarqubecloud
sonarqubecloudBot
commented
Nov 8, 2024

Copy link


Copy Markdown

Quality Gate PassedQuality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
@cgutman
cgutman
merged commit 1cab1a5
into
LizardByte:masterNov 8, 2024
qiin2333 referenced
this pull request
in AlkaidLab/foundation-sunshine
Nov 13, 2024
@cgutman@qiin2333
fix(process): avoid leaking zombies and fds in detached processes (#3…
f6d1b51
…379)

This was referenced Jan 18, 2025
 Merged

 Closed

 Merged

@ReenigneArcherReenigneArcher
mentioned this pull request
Jan 22, 2025
 Merged

c2vi pushed a commit
to c2vi/Sunshine
that referenced
this pull request
Mar 27, 2025
@cgutman@c2vi
fix(process): avoid leaking zombies and fds in detached processes (Li…
3c14121
…zardByte#3379)
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ReenigneArcherReenigneArcherReenigneArcher approved these changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Detached Processes launched by sunshine end up as zombies on linux after closing them


2 participants

@cgutman@ReenigneArcher

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix(process): avoid leaking zombies and fds in detached processes - #3379
Merged
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Nov 8, 2024
Merged
fix(process): avoid leaking zombies and fds in detached processes#3379
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Conversation
@cgutman
@cgutmancgutman
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
We were leaking detached child processes on Mac and Linux because we are expected to still wait on those with waitpid() in order to reap their entry in the process table. We will now do that when proc_t::running() is called (which happens regularly while streaming and periodically when clients poll for server info). This isn't a perfect solution but it's much better than the current situation today where all detached processes become zombies.
We were also leaking file descriptors into detached processes which could cause our log files and sockets to persist longer than expected.
Screenshot
Issues Fixed or Closed
Fixes#3378
Type of Change
  •  Bug fix (non-breaking change which fixes an issue)
  •  New feature (non-breaking change which adds functionality)
  •  Breaking change (fix or feature that would cause existing functionality to not work as expected)
  •  Dependency update (updates to dependencies)
  •  Documentation update (changes to documentation)
  •  Repository update (changes to repository files, e.g. .github/...)
Checklist
  •  My code follows the style guidelines of this project
  •  I have performed a self-review of my own code
  •  I have commented my code, particularly in hard-to-understand areas
  •  I have added or updated the in code docstring/documentation-blocks for new or existing methods/components
cgutman
cgutman
commented
Nov 8, 2024
View reviewed changes
Comment threadsrc/platform/linux/misc.cpp

 bp::child
 run_command(bool elevated, bool interactive, const std::string &cmd, boost::filesystem::path &working_dir, const bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) {
 // clang-format off
@cgutmancgutmanNov 8, 2024•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format gets confused by the boost::process syntax and thinks bp::std_in is a template class bp::std_in<bp::null, bp::std_out>
@codecov
codecovBot
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
Project coverage is 11.05%. Comparing base (54ddf37) to head (627b64f).

Report is 86 commits behind head on master.
Files with missing linesPatch %Lines
src/platform/linux/misc.cpp0.00%4 Missing ⚠️
src/platform/macos/misc.mm0.00%4 Missing ⚠️
src/process.cpp0.00%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3379 +/- ##
==========================================
- Coverage 11.11% 11.05% -0.06% 
==========================================
Files 75 99 +24 Lines 13204 17237 +4033 Branches 6084 8036 +1952 ==========================================
+ Hits 1467 1905 +438 - Misses 9307 12783 +3476 - Partials 2430 2549 +119 
FlagCoverage Δ
Linux8.34% <0.00%> (?)
Windows5.23% <ø> (ø)
macOS-1313.62% <0.00%> (-0.02%)⬇️
macOS-1412.62% <0.00%> (-0.01%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Files with missing linesCoverage Δ
src/process.cpp2.68% <0.00%> (+0.53%)⬆️
src/platform/linux/misc.cpp9.23% <0.00%> (ø)
src/platform/macos/misc.mm8.74% <0.00%> (-0.04%)⬇️
... and 44 files with indirect coverage changes
@sonarqubecloud
sonarqubecloudBot
commented
Nov 8, 2024

Copy link


Copy Markdown

Quality Gate PassedQuality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
@cgutman
cgutman
merged commit 1cab1a5
into
LizardByte:masterNov 8, 2024
qiin2333 referenced
this pull request
in AlkaidLab/foundation-sunshine
Nov 13, 2024
@cgutman@qiin2333
fix(process): avoid leaking zombies and fds in detached processes (#3…
f6d1b51
…379)

This was referenced Jan 18, 2025
 Merged

 Closed

 Merged

@ReenigneArcherReenigneArcher
mentioned this pull request
Jan 22, 2025
 Merged

c2vi pushed a commit
to c2vi/Sunshine
that referenced
this pull request
Mar 27, 2025
@cgutman@c2vi
fix(process): avoid leaking zombies and fds in detached processes (Li…
3c14121
…zardByte#3379)
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ReenigneArcherReenigneArcherReenigneArcher approved these changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Detached Processes launched by sunshine end up as zombies on linux after closing them


2 participants

@cgutman@ReenigneArcher

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(process): avoid leaking zombies and fds in detached processes - #3379
Merged
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Nov 8, 2024
Merged
fix(process): avoid leaking zombies and fds in detached processes#3379
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Conversation
@cgutman
@cgutmancgutman
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
We were leaking detached child processes on Mac and Linux because we are expected to still wait on those with waitpid() in order to reap their entry in the process table. We will now do that when proc_t::running() is called (which happens regularly while streaming and periodically when clients poll for server info). This isn't a perfect solution but it's much better than the current situation today where all detached processes become zombies.
We were also leaking file descriptors into detached processes which could cause our log files and sockets to persist longer than expected.
Screenshot
Issues Fixed or Closed
Fixes#3378
Type of Change
  •  Bug fix (non-breaking change which fixes an issue)
  •  New feature (non-breaking change which adds functionality)
  •  Breaking change (fix or feature that would cause existing functionality to not work as expected)
  •  Dependency update (updates to dependencies)
  •  Documentation update (changes to documentation)
  •  Repository update (changes to repository files, e.g. .github/...)
Checklist
  •  My code follows the style guidelines of this project
  •  I have performed a self-review of my own code
  •  I have commented my code, particularly in hard-to-understand areas
  •  I have added or updated the in code docstring/documentation-blocks for new or existing methods/components
cgutman
cgutman
commented
Nov 8, 2024
View reviewed changes
Comment threadsrc/platform/linux/misc.cpp

 bp::child
 run_command(bool elevated, bool interactive, const std::string &cmd, boost::filesystem::path &working_dir, const bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) {
 // clang-format off
@cgutmancgutmanNov 8, 2024•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format gets confused by the boost::process syntax and thinks bp::std_in is a template class bp::std_in<bp::null, bp::std_out>
@codecov
codecovBot
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
Project coverage is 11.05%. Comparing base (54ddf37) to head (627b64f).

Report is 86 commits behind head on master.
Files with missing linesPatch %Lines
src/platform/linux/misc.cpp0.00%4 Missing ⚠️
src/platform/macos/misc.mm0.00%4 Missing ⚠️
src/process.cpp0.00%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3379 +/- ##
==========================================
- Coverage 11.11% 11.05% -0.06% 
==========================================
Files 75 99 +24 Lines 13204 17237 +4033 Branches 6084 8036 +1952 ==========================================
+ Hits 1467 1905 +438 - Misses 9307 12783 +3476 - Partials 2430 2549 +119 
FlagCoverage Δ
Linux8.34% <0.00%> (?)
Windows5.23% <ø> (ø)
macOS-1313.62% <0.00%> (-0.02%)⬇️
macOS-1412.62% <0.00%> (-0.01%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Files with missing linesCoverage Δ
src/process.cpp2.68% <0.00%> (+0.53%)⬆️
src/platform/linux/misc.cpp9.23% <0.00%> (ø)
src/platform/macos/misc.mm8.74% <0.00%> (-0.04%)⬇️
... and 44 files with indirect coverage changes
@sonarqubecloud
sonarqubecloudBot
commented
Nov 8, 2024

Copy link


Copy Markdown

Quality Gate PassedQuality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
@cgutman
cgutman
merged commit 1cab1a5
into
LizardByte:masterNov 8, 2024
qiin2333 referenced
this pull request
in AlkaidLab/foundation-sunshine
Nov 13, 2024
@cgutman@qiin2333
fix(process): avoid leaking zombies and fds in detached processes (#3…
f6d1b51
…379)

This was referenced Jan 18, 2025
 Merged

 Closed

 Merged

@ReenigneArcherReenigneArcher
mentioned this pull request
Jan 22, 2025
 Merged

c2vi pushed a commit
to c2vi/Sunshine
that referenced
this pull request
Mar 27, 2025
@cgutman@c2vi
fix(process): avoid leaking zombies and fds in detached processes (Li…
3c14121
…zardByte#3379)
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ReenigneArcherReenigneArcherReenigneArcher approved these changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Detached Processes launched by sunshine end up as zombies on linux after closing them


2 participants

@cgutman@ReenigneArcher

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(process): avoid leaking zombies and fds in detached processes - #3379
Merged
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Nov 8, 2024
Merged
fix(process): avoid leaking zombies and fds in detached processes#3379
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Conversation
@cgutman
@cgutmancgutman
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
We were leaking detached child processes on Mac and Linux because we are expected to still wait on those with waitpid() in order to reap their entry in the process table. We will now do that when proc_t::running() is called (which happens regularly while streaming and periodically when clients poll for server info). This isn't a perfect solution but it's much better than the current situation today where all detached processes become zombies.
We were also leaking file descriptors into detached processes which could cause our log files and sockets to persist longer than expected.
Screenshot
Issues Fixed or Closed
Fixes#3378
Type of Change
  •  Bug fix (non-breaking change which fixes an issue)
  •  New feature (non-breaking change which adds functionality)
  •  Breaking change (fix or feature that would cause existing functionality to not work as expected)
  •  Dependency update (updates to dependencies)
  •  Documentation update (changes to documentation)
  •  Repository update (changes to repository files, e.g. .github/...)
Checklist
  •  My code follows the style guidelines of this project
  •  I have performed a self-review of my own code
  •  I have commented my code, particularly in hard-to-understand areas
  •  I have added or updated the in code docstring/documentation-blocks for new or existing methods/components
cgutman
cgutman
commented
Nov 8, 2024
View reviewed changes
Comment threadsrc/platform/linux/misc.cpp

 bp::child
 run_command(bool elevated, bool interactive, const std::string &cmd, boost::filesystem::path &working_dir, const bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) {
 // clang-format off
@cgutmancgutmanNov 8, 2024•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format gets confused by the boost::process syntax and thinks bp::std_in is a template class bp::std_in<bp::null, bp::std_out>
@codecov
codecovBot
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
Project coverage is 11.05%. Comparing base (54ddf37) to head (627b64f).

Report is 86 commits behind head on master.
Files with missing linesPatch %Lines
src/platform/linux/misc.cpp0.00%4 Missing ⚠️
src/platform/macos/misc.mm0.00%4 Missing ⚠️
src/process.cpp0.00%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3379 +/- ##
==========================================
- Coverage 11.11% 11.05% -0.06% 
==========================================
Files 75 99 +24 Lines 13204 17237 +4033 Branches 6084 8036 +1952 ==========================================
+ Hits 1467 1905 +438 - Misses 9307 12783 +3476 - Partials 2430 2549 +119 
FlagCoverage Δ
Linux8.34% <0.00%> (?)
Windows5.23% <ø> (ø)
macOS-1313.62% <0.00%> (-0.02%)⬇️
macOS-1412.62% <0.00%> (-0.01%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Files with missing linesCoverage Δ
src/process.cpp2.68% <0.00%> (+0.53%)⬆️
src/platform/linux/misc.cpp9.23% <0.00%> (ø)
src/platform/macos/misc.mm8.74% <0.00%> (-0.04%)⬇️
... and 44 files with indirect coverage changes
@sonarqubecloud
sonarqubecloudBot
commented
Nov 8, 2024

Copy link


Copy Markdown

Quality Gate PassedQuality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
@cgutman
cgutman
merged commit 1cab1a5
into
LizardByte:masterNov 8, 2024
qiin2333 referenced
this pull request
in AlkaidLab/foundation-sunshine
Nov 13, 2024
@cgutman@qiin2333
fix(process): avoid leaking zombies and fds in detached processes (#3…
f6d1b51
…379)

This was referenced Jan 18, 2025
 Merged

 Closed

 Merged

@ReenigneArcherReenigneArcher
mentioned this pull request
Jan 22, 2025
 Merged

c2vi pushed a commit
to c2vi/Sunshine
that referenced
this pull request
Mar 27, 2025
@cgutman@c2vi
fix(process): avoid leaking zombies and fds in detached processes (Li…
3c14121
…zardByte#3379)
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ReenigneArcherReenigneArcherReenigneArcher approved these changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Detached Processes launched by sunshine end up as zombies on linux after closing them


2 participants

@cgutman@ReenigneArcher

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix(process): avoid leaking zombies and fds in detached processes - #3379
Merged
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Nov 8, 2024
Merged
fix(process): avoid leaking zombies and fds in detached processes#3379
cgutman merged 1 commit into
LizardByte:masterfrom 
cgutman:zombie_leaks
Conversation
@cgutman
@cgutmancgutman
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
We were leaking detached child processes on Mac and Linux because we are expected to still wait on those with waitpid() in order to reap their entry in the process table. We will now do that when proc_t::running() is called (which happens regularly while streaming and periodically when clients poll for server info). This isn't a perfect solution but it's much better than the current situation today where all detached processes become zombies.
We were also leaking file descriptors into detached processes which could cause our log files and sockets to persist longer than expected.
Screenshot
Issues Fixed or Closed
Fixes#3378
Type of Change
  •  Bug fix (non-breaking change which fixes an issue)
  •  New feature (non-breaking change which adds functionality)
  •  Breaking change (fix or feature that would cause existing functionality to not work as expected)
  •  Dependency update (updates to dependencies)
  •  Documentation update (changes to documentation)
  •  Repository update (changes to repository files, e.g. .github/...)
Checklist
  •  My code follows the style guidelines of this project
  •  I have performed a self-review of my own code
  •  I have commented my code, particularly in hard-to-understand areas
  •  I have added or updated the in code docstring/documentation-blocks for new or existing methods/components
cgutman
cgutman
commented
Nov 8, 2024
View reviewed changes
Comment threadsrc/platform/linux/misc.cpp

 bp::child
 run_command(bool elevated, bool interactive, const std::string &cmd, boost::filesystem::path &working_dir, const bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) {
 // clang-format off
@cgutmancgutmanNov 8, 2024•

edited

Loading

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format gets confused by the boost::process syntax and thinks bp::std_in is a template class bp::std_in<bp::null, bp::std_out>
@codecov
codecovBot
commented
Nov 8, 2024•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
Project coverage is 11.05%. Comparing base (54ddf37) to head (627b64f).

Report is 86 commits behind head on master.
Files with missing linesPatch %Lines
src/platform/linux/misc.cpp0.00%4 Missing ⚠️
src/platform/macos/misc.mm0.00%4 Missing ⚠️
src/process.cpp0.00%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3379 +/- ##
==========================================
- Coverage 11.11% 11.05% -0.06% 
==========================================
Files 75 99 +24 Lines 13204 17237 +4033 Branches 6084 8036 +1952 ==========================================
+ Hits 1467 1905 +438 - Misses 9307 12783 +3476 - Partials 2430 2549 +119 
FlagCoverage Δ
Linux8.34% <0.00%> (?)
Windows5.23% <ø> (ø)
macOS-1313.62% <0.00%> (-0.02%)⬇️
macOS-1412.62% <0.00%> (-0.01%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Files with missing linesCoverage Δ
src/process.cpp2.68% <0.00%> (+0.53%)⬆️
src/platform/linux/misc.cpp9.23% <0.00%> (ø)
src/platform/macos/misc.mm8.74% <0.00%> (-0.04%)⬇️
... and 44 files with indirect coverage changes
@sonarqubecloud
sonarqubecloudBot
commented
Nov 8, 2024

Copy link


Copy Markdown

Quality Gate PassedQuality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarCloud
@cgutman
cgutman
merged commit 1cab1a5
into
LizardByte:masterNov 8, 2024
qiin2333 referenced
this pull request
in AlkaidLab/foundation-sunshine
Nov 13, 2024
@cgutman@qiin2333
fix(process): avoid leaking zombies and fds in detached processes (#3…
f6d1b51
…379)

This was referenced Jan 18, 2025
 Merged

 Closed

 Merged

@ReenigneArcherReenigneArcher
mentioned this pull request
Jan 22, 2025
 Merged

c2vi pushed a commit
to c2vi/Sunshine
that referenced
this pull request
Mar 27, 2025
@cgutman@c2vi
fix(process): avoid leaking zombies and fds in detached processes (Li…
3c14121
…zardByte#3379)
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ReenigneArcherReenigneArcherReenigneArcher approved these changes

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Detached Processes launched by sunshine end up as zombies on linux after closing them


2 participants

@cgutman@ReenigneArcher