[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530 - #103012

Merged
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0
Jun 4, 2024
Merged

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530#103012
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0

Conversation

@tommcdon

Copy link
Copy Markdown
Member

Backport of #102413 to release/6.0

If the call to CreateNamedPipe fails here:

ipc->pipe=CreateNamedPipeA (
ipc->pipe_name, // pipe name
creationFlags,
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
PIPE_UNLIMITED_INSTANCES, // max. instances
out_buffer_size, // output buffer size
in_buffer_size, // input buffer size
0, // default client time-out
NULL); // default security attribute
DS_EXIT_BLOCKING_PAL_SECTION;
if (ipc->pipe==INVALID_HANDLE_VALUE) {
if (callback)
callback ("Failed to create an instance of a named pipe.", GetLastError());
ep_raise_error ();
}

We will end up with an invalid handle for the pipe and the overlapped IO event. There currently is no code that tries to reset the connection and we will repeatedly try to poll an invalid handle, leading to one core being pegged as the wait fails due to an invalid handle and we keep waiting.

This PR makes it so we will call ds_ipc_listen when we run in to an error, which will reconnect to the named pipe. It also adds a delay when we detect an error, so if there are undetected cases that cause the same issue we will at least not consume the whole core we are running on.

Customer Impact

  • Customer reported
  • [] Found internally

The IPC server can fully consume a CPU core and prevent incoming connections.

Regression

  • [] Yes
  • No

Testing

Fix was verified in the customer's environment that it prevents the CPU issue

Risk

Low to Medium - The fix itself is small and reasonable, but this area of the code is hard to reason about all possible code paths due to the async nature of the named pipe APIs.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jun 3, 2024

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 6.0.x

@tommcdon

Copy link
Copy Markdown
MemberAuthor

All failures are known or infra related

@tommcdon
tommcdon merged commit 3f22874 into dotnet:release/6.0-stagingJun 4, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 5, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tommcdon@jeffschwMSFT@hoyosjs@davmason
, '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

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530 - #103012

Merged
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0
Jun 4, 2024
Merged

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530#103012
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0

Conversation

@tommcdon

Copy link
Copy Markdown
Member

Backport of #102413 to release/6.0

If the call to CreateNamedPipe fails here:

ipc->pipe=CreateNamedPipeA (
ipc->pipe_name, // pipe name
creationFlags,
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
PIPE_UNLIMITED_INSTANCES, // max. instances
out_buffer_size, // output buffer size
in_buffer_size, // input buffer size
0, // default client time-out
NULL); // default security attribute
DS_EXIT_BLOCKING_PAL_SECTION;
if (ipc->pipe==INVALID_HANDLE_VALUE) {
if (callback)
callback ("Failed to create an instance of a named pipe.", GetLastError());
ep_raise_error ();
}

We will end up with an invalid handle for the pipe and the overlapped IO event. There currently is no code that tries to reset the connection and we will repeatedly try to poll an invalid handle, leading to one core being pegged as the wait fails due to an invalid handle and we keep waiting.

This PR makes it so we will call ds_ipc_listen when we run in to an error, which will reconnect to the named pipe. It also adds a delay when we detect an error, so if there are undetected cases that cause the same issue we will at least not consume the whole core we are running on.

Customer Impact

  • Customer reported
  • [] Found internally

The IPC server can fully consume a CPU core and prevent incoming connections.

Regression

  • [] Yes
  • No

Testing

Fix was verified in the customer's environment that it prevents the CPU issue

Risk

Low to Medium - The fix itself is small and reasonable, but this area of the code is hard to reason about all possible code paths due to the async nature of the named pipe APIs.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jun 3, 2024

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 6.0.x

@tommcdon

Copy link
Copy Markdown
MemberAuthor

All failures are known or infra related

@tommcdon
tommcdon merged commit 3f22874 into dotnet:release/6.0-stagingJun 4, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 5, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tommcdon@jeffschwMSFT@hoyosjs@davmason
, '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

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530 - #103012

Merged
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0
Jun 4, 2024
Merged

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530#103012
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0

Conversation

@tommcdon

Copy link
Copy Markdown
Member

Backport of #102413 to release/6.0

If the call to CreateNamedPipe fails here:

ipc->pipe=CreateNamedPipeA (
ipc->pipe_name, // pipe name
creationFlags,
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
PIPE_UNLIMITED_INSTANCES, // max. instances
out_buffer_size, // output buffer size
in_buffer_size, // input buffer size
0, // default client time-out
NULL); // default security attribute
DS_EXIT_BLOCKING_PAL_SECTION;
if (ipc->pipe==INVALID_HANDLE_VALUE) {
if (callback)
callback ("Failed to create an instance of a named pipe.", GetLastError());
ep_raise_error ();
}

We will end up with an invalid handle for the pipe and the overlapped IO event. There currently is no code that tries to reset the connection and we will repeatedly try to poll an invalid handle, leading to one core being pegged as the wait fails due to an invalid handle and we keep waiting.

This PR makes it so we will call ds_ipc_listen when we run in to an error, which will reconnect to the named pipe. It also adds a delay when we detect an error, so if there are undetected cases that cause the same issue we will at least not consume the whole core we are running on.

Customer Impact

  • Customer reported
  • [] Found internally

The IPC server can fully consume a CPU core and prevent incoming connections.

Regression

  • [] Yes
  • No

Testing

Fix was verified in the customer's environment that it prevents the CPU issue

Risk

Low to Medium - The fix itself is small and reasonable, but this area of the code is hard to reason about all possible code paths due to the async nature of the named pipe APIs.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jun 3, 2024

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 6.0.x

@tommcdon

Copy link
Copy Markdown
MemberAuthor

All failures are known or infra related

@tommcdon
tommcdon merged commit 3f22874 into dotnet:release/6.0-stagingJun 4, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 5, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tommcdon@jeffschwMSFT@hoyosjs@davmason
, '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

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530 - #103012

Merged
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0
Jun 4, 2024
Merged

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530#103012
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0

Conversation

@tommcdon

Copy link
Copy Markdown
Member

Backport of #102413 to release/6.0

If the call to CreateNamedPipe fails here:

ipc->pipe=CreateNamedPipeA (
ipc->pipe_name, // pipe name
creationFlags,
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
PIPE_UNLIMITED_INSTANCES, // max. instances
out_buffer_size, // output buffer size
in_buffer_size, // input buffer size
0, // default client time-out
NULL); // default security attribute
DS_EXIT_BLOCKING_PAL_SECTION;
if (ipc->pipe==INVALID_HANDLE_VALUE) {
if (callback)
callback ("Failed to create an instance of a named pipe.", GetLastError());
ep_raise_error ();
}

We will end up with an invalid handle for the pipe and the overlapped IO event. There currently is no code that tries to reset the connection and we will repeatedly try to poll an invalid handle, leading to one core being pegged as the wait fails due to an invalid handle and we keep waiting.

This PR makes it so we will call ds_ipc_listen when we run in to an error, which will reconnect to the named pipe. It also adds a delay when we detect an error, so if there are undetected cases that cause the same issue we will at least not consume the whole core we are running on.

Customer Impact

  • Customer reported
  • [] Found internally

The IPC server can fully consume a CPU core and prevent incoming connections.

Regression

  • [] Yes
  • No

Testing

Fix was verified in the customer's environment that it prevents the CPU issue

Risk

Low to Medium - The fix itself is small and reasonable, but this area of the code is hard to reason about all possible code paths due to the async nature of the named pipe APIs.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jun 3, 2024

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 6.0.x

@tommcdon

Copy link
Copy Markdown
MemberAuthor

All failures are known or infra related

@tommcdon
tommcdon merged commit 3f22874 into dotnet:release/6.0-stagingJun 4, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 5, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tommcdon@jeffschwMSFT@hoyosjs@davmason
, '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

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530 - #103012

Merged
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0
Jun 4, 2024
Merged

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530#103012
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0

Conversation

@tommcdon

Copy link
Copy Markdown
Member

Backport of #102413 to release/6.0

If the call to CreateNamedPipe fails here:

ipc->pipe=CreateNamedPipeA (
ipc->pipe_name, // pipe name
creationFlags,
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
PIPE_UNLIMITED_INSTANCES, // max. instances
out_buffer_size, // output buffer size
in_buffer_size, // input buffer size
0, // default client time-out
NULL); // default security attribute
DS_EXIT_BLOCKING_PAL_SECTION;
if (ipc->pipe==INVALID_HANDLE_VALUE) {
if (callback)
callback ("Failed to create an instance of a named pipe.", GetLastError());
ep_raise_error ();
}

We will end up with an invalid handle for the pipe and the overlapped IO event. There currently is no code that tries to reset the connection and we will repeatedly try to poll an invalid handle, leading to one core being pegged as the wait fails due to an invalid handle and we keep waiting.

This PR makes it so we will call ds_ipc_listen when we run in to an error, which will reconnect to the named pipe. It also adds a delay when we detect an error, so if there are undetected cases that cause the same issue we will at least not consume the whole core we are running on.

Customer Impact

  • Customer reported
  • [] Found internally

The IPC server can fully consume a CPU core and prevent incoming connections.

Regression

  • [] Yes
  • No

Testing

Fix was verified in the customer's environment that it prevents the CPU issue

Risk

Low to Medium - The fix itself is small and reasonable, but this area of the code is hard to reason about all possible code paths due to the async nature of the named pipe APIs.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jun 3, 2024

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 6.0.x

@tommcdon

Copy link
Copy Markdown
MemberAuthor

All failures are known or infra related

@tommcdon
tommcdon merged commit 3f22874 into dotnet:release/6.0-stagingJun 4, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 5, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tommcdon@jeffschwMSFT@hoyosjs@davmason
, '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

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530 - #103012

Merged
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0
Jun 4, 2024
Merged

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530#103012
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0

Conversation

@tommcdon

Copy link
Copy Markdown
Member

Backport of #102413 to release/6.0

If the call to CreateNamedPipe fails here:

ipc->pipe=CreateNamedPipeA (
ipc->pipe_name, // pipe name
creationFlags,
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
PIPE_UNLIMITED_INSTANCES, // max. instances
out_buffer_size, // output buffer size
in_buffer_size, // input buffer size
0, // default client time-out
NULL); // default security attribute
DS_EXIT_BLOCKING_PAL_SECTION;
if (ipc->pipe==INVALID_HANDLE_VALUE) {
if (callback)
callback ("Failed to create an instance of a named pipe.", GetLastError());
ep_raise_error ();
}

We will end up with an invalid handle for the pipe and the overlapped IO event. There currently is no code that tries to reset the connection and we will repeatedly try to poll an invalid handle, leading to one core being pegged as the wait fails due to an invalid handle and we keep waiting.

This PR makes it so we will call ds_ipc_listen when we run in to an error, which will reconnect to the named pipe. It also adds a delay when we detect an error, so if there are undetected cases that cause the same issue we will at least not consume the whole core we are running on.

Customer Impact

  • Customer reported
  • [] Found internally

The IPC server can fully consume a CPU core and prevent incoming connections.

Regression

  • [] Yes
  • No

Testing

Fix was verified in the customer's environment that it prevents the CPU issue

Risk

Low to Medium - The fix itself is small and reasonable, but this area of the code is hard to reason about all possible code paths due to the async nature of the named pipe APIs.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jun 3, 2024

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 6.0.x

@tommcdon

Copy link
Copy Markdown
MemberAuthor

All failures are known or infra related

@tommcdon
tommcdon merged commit 3f22874 into dotnet:release/6.0-stagingJun 4, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 5, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tommcdon@jeffschwMSFT@hoyosjs@davmason
, '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

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530 - #103012

Merged
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0
Jun 4, 2024
Merged

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530#103012
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0

Conversation

@tommcdon

Copy link
Copy Markdown
Member

Backport of #102413 to release/6.0

If the call to CreateNamedPipe fails here:

ipc->pipe=CreateNamedPipeA (
ipc->pipe_name, // pipe name
creationFlags,
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
PIPE_UNLIMITED_INSTANCES, // max. instances
out_buffer_size, // output buffer size
in_buffer_size, // input buffer size
0, // default client time-out
NULL); // default security attribute
DS_EXIT_BLOCKING_PAL_SECTION;
if (ipc->pipe==INVALID_HANDLE_VALUE) {
if (callback)
callback ("Failed to create an instance of a named pipe.", GetLastError());
ep_raise_error ();
}

We will end up with an invalid handle for the pipe and the overlapped IO event. There currently is no code that tries to reset the connection and we will repeatedly try to poll an invalid handle, leading to one core being pegged as the wait fails due to an invalid handle and we keep waiting.

This PR makes it so we will call ds_ipc_listen when we run in to an error, which will reconnect to the named pipe. It also adds a delay when we detect an error, so if there are undetected cases that cause the same issue we will at least not consume the whole core we are running on.

Customer Impact

  • Customer reported
  • [] Found internally

The IPC server can fully consume a CPU core and prevent incoming connections.

Regression

  • [] Yes
  • No

Testing

Fix was verified in the customer's environment that it prevents the CPU issue

Risk

Low to Medium - The fix itself is small and reasonable, but this area of the code is hard to reason about all possible code paths due to the async nature of the named pipe APIs.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jun 3, 2024

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 6.0.x

@tommcdon

Copy link
Copy Markdown
MemberAuthor

All failures are known or infra related

@tommcdon
tommcdon merged commit 3f22874 into dotnet:release/6.0-stagingJun 4, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 5, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tommcdon@jeffschwMSFT@hoyosjs@davmason
, '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

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530 - #103012

Merged
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0
Jun 4, 2024
Merged

[Release/6.0-staging] Fix issue where the IPC server can fully consume a CPU core and prevent incoming connections #102530#103012
tommcdon merged 2 commits into
dotnet:release/6.0-stagingfrom
tommcdon:dev/tommcdon/ipc_bug_6.0

Conversation

@tommcdon

Copy link
Copy Markdown
Member

Backport of #102413 to release/6.0

If the call to CreateNamedPipe fails here:

ipc->pipe=CreateNamedPipeA (
ipc->pipe_name, // pipe name
creationFlags,
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
PIPE_UNLIMITED_INSTANCES, // max. instances
out_buffer_size, // output buffer size
in_buffer_size, // input buffer size
0, // default client time-out
NULL); // default security attribute
DS_EXIT_BLOCKING_PAL_SECTION;
if (ipc->pipe==INVALID_HANDLE_VALUE) {
if (callback)
callback ("Failed to create an instance of a named pipe.", GetLastError());
ep_raise_error ();
}

We will end up with an invalid handle for the pipe and the overlapped IO event. There currently is no code that tries to reset the connection and we will repeatedly try to poll an invalid handle, leading to one core being pegged as the wait fails due to an invalid handle and we keep waiting.

This PR makes it so we will call ds_ipc_listen when we run in to an error, which will reconnect to the named pipe. It also adds a delay when we detect an error, so if there are undetected cases that cause the same issue we will at least not consume the whole core we are running on.

Customer Impact

  • Customer reported
  • [] Found internally

The IPC server can fully consume a CPU core and prevent incoming connections.

Regression

  • [] Yes
  • No

Testing

Fix was verified in the customer's environment that it prevents the CPU issue

Risk

Low to Medium - The fix itself is small and reasonable, but this area of the code is hard to reason about all possible code paths due to the async nature of the named pipe APIs.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jun 3, 2024

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 6.0.x

@tommcdon

Copy link
Copy Markdown
MemberAuthor

All failures are known or infra related

@tommcdon
tommcdon merged commit 3f22874 into dotnet:release/6.0-stagingJun 4, 2024
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 5, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tommcdon@jeffschwMSFT@hoyosjs@davmason