System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed - #109799

Merged
ManickaP merged 3 commits into
dotnet:mainfrom
pedrobsaila:83486
Feb 3, 2025
Merged

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed#109799
ManickaP merged 3 commits into
dotnet:mainfrom
pedrobsaila:83486

Conversation

@pedrobsaila

@pedrobsailapedrobsaila commented Nov 13, 2024

Copy link
Copy Markdown
Contributor

Fixes#83486

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 13, 2024
This was referenced Nov 14, 2024
Closed
Closed
@CarnaViireCarnaViire self-assigned this Nov 14, 2024
liveans
liveans previously approved these changes Dec 9, 2024
View reviewed changes

@liveansliveans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@karelzkarelz added this to the 10.0.0 milestone Dec 9, 2024
@liveans
liveans dismissed their stale reviewDecember 9, 2024 14:48

Natalia had some concerns, need to deep-dive a bit more

@karelz

karelz commented Dec 17, 2024

Copy link
Copy Markdown
Member

@pedrobsaila FYI - People disappeared for the holidays, so we will not be able to get back to it until January. Hopefully it is alright. Sorry for the inconvenience!

@pedrobsaila

pedrobsaila commented Dec 17, 2024

Copy link
Copy Markdown
ContributorAuthor

no worries @karelz happy holidays to all the team

@CarnaViire

CarnaViire commented Jan 14, 2025

Copy link
Copy Markdown
Member

Sorry for the delay. Now I'm back from vacation, so I will be on it shortly.

@ManickaP

ManickaP commented Jan 30, 2025

Copy link
Copy Markdown
Member

There's also another place with DisposeAndClearHandle:

runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Line 1036 in 99b373a

SafeWinHttpHandle.DisposeAndClearHandle(refconnectHandle);

Could you also replace it with an ordinary dispose - it's on a local variable so not nulling won't affect anything. So that we are consistent in how we dispose handles.

Lastly, can we use Interlocked.CompareExchange in Dispose:

runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines 542 to 544 in 99b373a

if(!_disposed)
{
_disposed=true;

to be 100% sure that we don't call dispose twice on the session handle in some obscure race condition (which could mess up reference counting for the handle).

Otherwise, LGTM. And apologies for taking this long with the review. If you've already forgotten about this PR, I can take over and work in feedback.

@build-analysisbuild-analysisBot mentioned this pull request Feb 2, 2025
Closed
ManickaP
ManickaP approved these changes Feb 3, 2025
View reviewed changes

@ManickaPManickaP 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, thank you very much for the contribution and patience.

@ManickaP
ManickaP merged commit ad382ca into dotnet:mainFeb 3, 2025
@pedrobsaila
pedrobsaila deleted the 83486 branch February 3, 2025 10:04
grendello added a commit to grendello/runtime that referenced this pull request Feb 3, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
28eb88e
* main:
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
Keep test PDB in helix payload for native AOT (dotnet#111949)
Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
Always inline number conversions (dotnet#112061)
Use Contains{Any} in Regex source generator (dotnet#112065)
Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
JIT: Update type when return temp is freshly created (dotnet#111948)
Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
WasmAppBuilder: Remove double computation of a value (dotnet#112047)
Disable LTCG for brotli and zlibng. (dotnet#111805)
JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
simplify x86 special intrinsic imports (dotnet#111836)
JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
Fix explicit offset of ByRefLike fields. (dotnet#111584)
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Mar 6, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ManickaPManickaPManickaP approved these changes
+1 more reviewer
@liveansliveansliveans left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Httpcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed

5 participants

@pedrobsaila@karelz@CarnaViire@ManickaP@liveans
, '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
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed - #109799
Merged
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Feb 3, 2025
Merged
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed#109799
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Conversation
@pedrobsaila
@pedrobsailapedrobsaila
commented
Nov 13, 2024

Copy link


Copy Markdown

Contributor
Fixes#83486
@dotnet-policy-servicedotnet-policy-serviceBot
added
the 
community-contribution
Indicates that the PR has been added by a community member
label
Nov 13, 2024

This was referenced Nov 14, 2024

Closed


Closed

@CarnaViireCarnaViire
self-assigned this
Nov 14, 2024
liveans
liveans
previously approved these changes
Dec 9, 2024
View reviewed changes
@liveansliveans
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@karelzkarelz
added this to the 10.0.0 milestone
Dec 9, 2024
@liveans
liveans
dismissed
their stale reviewDecember 9, 2024 14:48
Natalia had some concerns, need to deep-dive a bit more
@karelz
karelz
commented
Dec 17, 2024

Copy link


Copy Markdown

Member
@pedrobsaila FYI - People disappeared for the holidays, so we will not be able to get back to it until January. Hopefully it is alright. Sorry for the inconvenience!
@pedrobsaila
pedrobsaila
commented
Dec 17, 2024

Copy link


Copy Markdown

ContributorAuthor
no worries @karelz happy holidays to all the team
@CarnaViire
CarnaViire
commented
Jan 14, 2025

Copy link


Copy Markdown

Member
Sorry for the delay. Now I'm back from vacation, so I will be on it shortly.
@ManickaP
ManickaP
commented
Jan 30, 2025

Copy link


Copy Markdown

Member
There's also another place with DisposeAndClearHandle:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Line 1036
in
99b373a
SafeWinHttpHandle.DisposeAndClearHandle(refconnectHandle);
Could you also replace it with an ordinary dispose - it's on a local variable so not nulling won't affect anything. So that we are consistent in how we dispose handles.
Lastly, can we use Interlocked.CompareExchange in Dispose:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines 542 to 544
in
99b373a
if(!_disposed)
{
_disposed=true;


to be 100% sure that we don't call dispose twice on the session handle in some obscure race condition (which could mess up reference counting for the handle).
Otherwise, LGTM. And apologies for taking this long with the review. If you've already forgotten about this PR, I can take over and work in feedback.
@build-analysisbuild-analysisBot
mentioned this pull request
Feb 2, 2025

Closed

ManickaP
ManickaP
approved these changes
Feb 3, 2025
View reviewed changes
@ManickaPManickaP
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, thank you very much for the contribution and patience.
@ManickaP
ManickaP
merged commit ad382ca
into
dotnet:mainFeb 3, 2025
@pedrobsaila
pedrobsaila
deleted the

83486

branch
February 3, 2025 10:04
grendello added a commit
to grendello/runtime
that referenced
this pull request
Feb 3, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
28eb88e
* main:
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
Keep test PDB in helix payload for native AOT (dotnet#111949)
Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
Always inline number conversions (dotnet#112061)
Use Contains{Any} in Regex source generator (dotnet#112065)
Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
JIT: Update type when return temp is freshly created (dotnet#111948)
Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
WasmAppBuilder: Remove double computation of a value (dotnet#112047)
Disable LTCG for brotli and zlibng. (dotnet#111805)
JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
simplify x86 special intrinsic imports (dotnet#111836)
JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
Fix explicit offset of ByRefLike fields. (dotnet#111584)
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 6, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@ManickaPManickaPManickaP approved these changes

+1 more reviewer

@liveansliveansliveans left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Httpcommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed


5 participants

@pedrobsaila@karelz@CarnaViire@ManickaP@liveans

, '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
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed - #109799
Merged
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Feb 3, 2025
Merged
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed#109799
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Conversation
@pedrobsaila
@pedrobsailapedrobsaila
commented
Nov 13, 2024

Copy link


Copy Markdown

Contributor
Fixes#83486
@dotnet-policy-servicedotnet-policy-serviceBot
added
the 
community-contribution
Indicates that the PR has been added by a community member
label
Nov 13, 2024

This was referenced Nov 14, 2024

Closed


Closed

@CarnaViireCarnaViire
self-assigned this
Nov 14, 2024
liveans
liveans
previously approved these changes
Dec 9, 2024
View reviewed changes
@liveansliveans
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@karelzkarelz
added this to the 10.0.0 milestone
Dec 9, 2024
@liveans
liveans
dismissed
their stale reviewDecember 9, 2024 14:48
Natalia had some concerns, need to deep-dive a bit more
@karelz
karelz
commented
Dec 17, 2024

Copy link


Copy Markdown

Member
@pedrobsaila FYI - People disappeared for the holidays, so we will not be able to get back to it until January. Hopefully it is alright. Sorry for the inconvenience!
@pedrobsaila
pedrobsaila
commented
Dec 17, 2024

Copy link


Copy Markdown

ContributorAuthor
no worries @karelz happy holidays to all the team
@CarnaViire
CarnaViire
commented
Jan 14, 2025

Copy link


Copy Markdown

Member
Sorry for the delay. Now I'm back from vacation, so I will be on it shortly.
@ManickaP
ManickaP
commented
Jan 30, 2025

Copy link


Copy Markdown

Member
There's also another place with DisposeAndClearHandle:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Line 1036
in
99b373a
SafeWinHttpHandle.DisposeAndClearHandle(refconnectHandle);
Could you also replace it with an ordinary dispose - it's on a local variable so not nulling won't affect anything. So that we are consistent in how we dispose handles.
Lastly, can we use Interlocked.CompareExchange in Dispose:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines 542 to 544
in
99b373a
if(!_disposed)
{
_disposed=true;


to be 100% sure that we don't call dispose twice on the session handle in some obscure race condition (which could mess up reference counting for the handle).
Otherwise, LGTM. And apologies for taking this long with the review. If you've already forgotten about this PR, I can take over and work in feedback.
@build-analysisbuild-analysisBot
mentioned this pull request
Feb 2, 2025

Closed

ManickaP
ManickaP
approved these changes
Feb 3, 2025
View reviewed changes
@ManickaPManickaP
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, thank you very much for the contribution and patience.
@ManickaP
ManickaP
merged commit ad382ca
into
dotnet:mainFeb 3, 2025
@pedrobsaila
pedrobsaila
deleted the

83486

branch
February 3, 2025 10:04
grendello added a commit
to grendello/runtime
that referenced
this pull request
Feb 3, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
28eb88e
* main:
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
Keep test PDB in helix payload for native AOT (dotnet#111949)
Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
Always inline number conversions (dotnet#112061)
Use Contains{Any} in Regex source generator (dotnet#112065)
Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
JIT: Update type when return temp is freshly created (dotnet#111948)
Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
WasmAppBuilder: Remove double computation of a value (dotnet#112047)
Disable LTCG for brotli and zlibng. (dotnet#111805)
JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
simplify x86 special intrinsic imports (dotnet#111836)
JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
Fix explicit offset of ByRefLike fields. (dotnet#111584)
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 6, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@ManickaPManickaPManickaP approved these changes

+1 more reviewer

@liveansliveansliveans left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Httpcommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed


5 participants

@pedrobsaila@karelz@CarnaViire@ManickaP@liveans

, '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
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed - #109799
Merged
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Feb 3, 2025
Merged
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed#109799
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Conversation
@pedrobsaila
@pedrobsailapedrobsaila
commented
Nov 13, 2024

Copy link


Copy Markdown

Contributor
Fixes#83486
@dotnet-policy-servicedotnet-policy-serviceBot
added
the 
community-contribution
Indicates that the PR has been added by a community member
label
Nov 13, 2024

This was referenced Nov 14, 2024

Closed


Closed

@CarnaViireCarnaViire
self-assigned this
Nov 14, 2024
liveans
liveans
previously approved these changes
Dec 9, 2024
View reviewed changes
@liveansliveans
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@karelzkarelz
added this to the 10.0.0 milestone
Dec 9, 2024
@liveans
liveans
dismissed
their stale reviewDecember 9, 2024 14:48
Natalia had some concerns, need to deep-dive a bit more
@karelz
karelz
commented
Dec 17, 2024

Copy link


Copy Markdown

Member
@pedrobsaila FYI - People disappeared for the holidays, so we will not be able to get back to it until January. Hopefully it is alright. Sorry for the inconvenience!
@pedrobsaila
pedrobsaila
commented
Dec 17, 2024

Copy link


Copy Markdown

ContributorAuthor
no worries @karelz happy holidays to all the team
@CarnaViire
CarnaViire
commented
Jan 14, 2025

Copy link


Copy Markdown

Member
Sorry for the delay. Now I'm back from vacation, so I will be on it shortly.
@ManickaP
ManickaP
commented
Jan 30, 2025

Copy link


Copy Markdown

Member
There's also another place with DisposeAndClearHandle:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Line 1036
in
99b373a
SafeWinHttpHandle.DisposeAndClearHandle(refconnectHandle);
Could you also replace it with an ordinary dispose - it's on a local variable so not nulling won't affect anything. So that we are consistent in how we dispose handles.
Lastly, can we use Interlocked.CompareExchange in Dispose:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines 542 to 544
in
99b373a
if(!_disposed)
{
_disposed=true;


to be 100% sure that we don't call dispose twice on the session handle in some obscure race condition (which could mess up reference counting for the handle).
Otherwise, LGTM. And apologies for taking this long with the review. If you've already forgotten about this PR, I can take over and work in feedback.
@build-analysisbuild-analysisBot
mentioned this pull request
Feb 2, 2025

Closed

ManickaP
ManickaP
approved these changes
Feb 3, 2025
View reviewed changes
@ManickaPManickaP
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, thank you very much for the contribution and patience.
@ManickaP
ManickaP
merged commit ad382ca
into
dotnet:mainFeb 3, 2025
@pedrobsaila
pedrobsaila
deleted the

83486

branch
February 3, 2025 10:04
grendello added a commit
to grendello/runtime
that referenced
this pull request
Feb 3, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
28eb88e
* main:
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
Keep test PDB in helix payload for native AOT (dotnet#111949)
Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
Always inline number conversions (dotnet#112061)
Use Contains{Any} in Regex source generator (dotnet#112065)
Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
JIT: Update type when return temp is freshly created (dotnet#111948)
Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
WasmAppBuilder: Remove double computation of a value (dotnet#112047)
Disable LTCG for brotli and zlibng. (dotnet#111805)
JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
simplify x86 special intrinsic imports (dotnet#111836)
JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
Fix explicit offset of ByRefLike fields. (dotnet#111584)
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 6, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@ManickaPManickaPManickaP approved these changes

+1 more reviewer

@liveansliveansliveans left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Httpcommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed


5 participants

@pedrobsaila@karelz@CarnaViire@ManickaP@liveans

, '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
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed - #109799
Merged
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Feb 3, 2025
Merged
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed#109799
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Conversation
@pedrobsaila
@pedrobsailapedrobsaila
commented
Nov 13, 2024

Copy link


Copy Markdown

Contributor
Fixes#83486
@dotnet-policy-servicedotnet-policy-serviceBot
added
the 
community-contribution
Indicates that the PR has been added by a community member
label
Nov 13, 2024

This was referenced Nov 14, 2024

Closed


Closed

@CarnaViireCarnaViire
self-assigned this
Nov 14, 2024
liveans
liveans
previously approved these changes
Dec 9, 2024
View reviewed changes
@liveansliveans
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@karelzkarelz
added this to the 10.0.0 milestone
Dec 9, 2024
@liveans
liveans
dismissed
their stale reviewDecember 9, 2024 14:48
Natalia had some concerns, need to deep-dive a bit more
@karelz
karelz
commented
Dec 17, 2024

Copy link


Copy Markdown

Member
@pedrobsaila FYI - People disappeared for the holidays, so we will not be able to get back to it until January. Hopefully it is alright. Sorry for the inconvenience!
@pedrobsaila
pedrobsaila
commented
Dec 17, 2024

Copy link


Copy Markdown

ContributorAuthor
no worries @karelz happy holidays to all the team
@CarnaViire
CarnaViire
commented
Jan 14, 2025

Copy link


Copy Markdown

Member
Sorry for the delay. Now I'm back from vacation, so I will be on it shortly.
@ManickaP
ManickaP
commented
Jan 30, 2025

Copy link


Copy Markdown

Member
There's also another place with DisposeAndClearHandle:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Line 1036
in
99b373a
SafeWinHttpHandle.DisposeAndClearHandle(refconnectHandle);
Could you also replace it with an ordinary dispose - it's on a local variable so not nulling won't affect anything. So that we are consistent in how we dispose handles.
Lastly, can we use Interlocked.CompareExchange in Dispose:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines 542 to 544
in
99b373a
if(!_disposed)
{
_disposed=true;


to be 100% sure that we don't call dispose twice on the session handle in some obscure race condition (which could mess up reference counting for the handle).
Otherwise, LGTM. And apologies for taking this long with the review. If you've already forgotten about this PR, I can take over and work in feedback.
@build-analysisbuild-analysisBot
mentioned this pull request
Feb 2, 2025

Closed

ManickaP
ManickaP
approved these changes
Feb 3, 2025
View reviewed changes
@ManickaPManickaP
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, thank you very much for the contribution and patience.
@ManickaP
ManickaP
merged commit ad382ca
into
dotnet:mainFeb 3, 2025
@pedrobsaila
pedrobsaila
deleted the

83486

branch
February 3, 2025 10:04
grendello added a commit
to grendello/runtime
that referenced
this pull request
Feb 3, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
28eb88e
* main:
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
Keep test PDB in helix payload for native AOT (dotnet#111949)
Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
Always inline number conversions (dotnet#112061)
Use Contains{Any} in Regex source generator (dotnet#112065)
Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
JIT: Update type when return temp is freshly created (dotnet#111948)
Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
WasmAppBuilder: Remove double computation of a value (dotnet#112047)
Disable LTCG for brotli and zlibng. (dotnet#111805)
JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
simplify x86 special intrinsic imports (dotnet#111836)
JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
Fix explicit offset of ByRefLike fields. (dotnet#111584)
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 6, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@ManickaPManickaPManickaP approved these changes

+1 more reviewer

@liveansliveansliveans left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Httpcommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed


5 participants

@pedrobsaila@karelz@CarnaViire@ManickaP@liveans

, '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
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed - #109799
Merged
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Feb 3, 2025
Merged
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed#109799
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Conversation
@pedrobsaila
@pedrobsailapedrobsaila
commented
Nov 13, 2024

Copy link


Copy Markdown

Contributor
Fixes#83486
@dotnet-policy-servicedotnet-policy-serviceBot
added
the 
community-contribution
Indicates that the PR has been added by a community member
label
Nov 13, 2024

This was referenced Nov 14, 2024

Closed


Closed

@CarnaViireCarnaViire
self-assigned this
Nov 14, 2024
liveans
liveans
previously approved these changes
Dec 9, 2024
View reviewed changes
@liveansliveans
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@karelzkarelz
added this to the 10.0.0 milestone
Dec 9, 2024
@liveans
liveans
dismissed
their stale reviewDecember 9, 2024 14:48
Natalia had some concerns, need to deep-dive a bit more
@karelz
karelz
commented
Dec 17, 2024

Copy link


Copy Markdown

Member
@pedrobsaila FYI - People disappeared for the holidays, so we will not be able to get back to it until January. Hopefully it is alright. Sorry for the inconvenience!
@pedrobsaila
pedrobsaila
commented
Dec 17, 2024

Copy link


Copy Markdown

ContributorAuthor
no worries @karelz happy holidays to all the team
@CarnaViire
CarnaViire
commented
Jan 14, 2025

Copy link


Copy Markdown

Member
Sorry for the delay. Now I'm back from vacation, so I will be on it shortly.
@ManickaP
ManickaP
commented
Jan 30, 2025

Copy link


Copy Markdown

Member
There's also another place with DisposeAndClearHandle:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Line 1036
in
99b373a
SafeWinHttpHandle.DisposeAndClearHandle(refconnectHandle);
Could you also replace it with an ordinary dispose - it's on a local variable so not nulling won't affect anything. So that we are consistent in how we dispose handles.
Lastly, can we use Interlocked.CompareExchange in Dispose:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines 542 to 544
in
99b373a
if(!_disposed)
{
_disposed=true;


to be 100% sure that we don't call dispose twice on the session handle in some obscure race condition (which could mess up reference counting for the handle).
Otherwise, LGTM. And apologies for taking this long with the review. If you've already forgotten about this PR, I can take over and work in feedback.
@build-analysisbuild-analysisBot
mentioned this pull request
Feb 2, 2025

Closed

ManickaP
ManickaP
approved these changes
Feb 3, 2025
View reviewed changes
@ManickaPManickaP
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, thank you very much for the contribution and patience.
@ManickaP
ManickaP
merged commit ad382ca
into
dotnet:mainFeb 3, 2025
@pedrobsaila
pedrobsaila
deleted the

83486

branch
February 3, 2025 10:04
grendello added a commit
to grendello/runtime
that referenced
this pull request
Feb 3, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
28eb88e
* main:
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
Keep test PDB in helix payload for native AOT (dotnet#111949)
Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
Always inline number conversions (dotnet#112061)
Use Contains{Any} in Regex source generator (dotnet#112065)
Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
JIT: Update type when return temp is freshly created (dotnet#111948)
Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
WasmAppBuilder: Remove double computation of a value (dotnet#112047)
Disable LTCG for brotli and zlibng. (dotnet#111805)
JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
simplify x86 special intrinsic imports (dotnet#111836)
JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
Fix explicit offset of ByRefLike fields. (dotnet#111584)
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 6, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@ManickaPManickaPManickaP approved these changes

+1 more reviewer

@liveansliveansliveans left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Httpcommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed


5 participants

@pedrobsaila@karelz@CarnaViire@ManickaP@liveans

, '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
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed - #109799
Merged
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Feb 3, 2025
Merged
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed#109799
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Conversation
@pedrobsaila
@pedrobsailapedrobsaila
commented
Nov 13, 2024

Copy link


Copy Markdown

Contributor
Fixes#83486
@dotnet-policy-servicedotnet-policy-serviceBot
added
the 
community-contribution
Indicates that the PR has been added by a community member
label
Nov 13, 2024

This was referenced Nov 14, 2024

Closed


Closed

@CarnaViireCarnaViire
self-assigned this
Nov 14, 2024
liveans
liveans
previously approved these changes
Dec 9, 2024
View reviewed changes
@liveansliveans
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@karelzkarelz
added this to the 10.0.0 milestone
Dec 9, 2024
@liveans
liveans
dismissed
their stale reviewDecember 9, 2024 14:48
Natalia had some concerns, need to deep-dive a bit more
@karelz
karelz
commented
Dec 17, 2024

Copy link


Copy Markdown

Member
@pedrobsaila FYI - People disappeared for the holidays, so we will not be able to get back to it until January. Hopefully it is alright. Sorry for the inconvenience!
@pedrobsaila
pedrobsaila
commented
Dec 17, 2024

Copy link


Copy Markdown

ContributorAuthor
no worries @karelz happy holidays to all the team
@CarnaViire
CarnaViire
commented
Jan 14, 2025

Copy link


Copy Markdown

Member
Sorry for the delay. Now I'm back from vacation, so I will be on it shortly.
@ManickaP
ManickaP
commented
Jan 30, 2025

Copy link


Copy Markdown

Member
There's also another place with DisposeAndClearHandle:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Line 1036
in
99b373a
SafeWinHttpHandle.DisposeAndClearHandle(refconnectHandle);
Could you also replace it with an ordinary dispose - it's on a local variable so not nulling won't affect anything. So that we are consistent in how we dispose handles.
Lastly, can we use Interlocked.CompareExchange in Dispose:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines 542 to 544
in
99b373a
if(!_disposed)
{
_disposed=true;


to be 100% sure that we don't call dispose twice on the session handle in some obscure race condition (which could mess up reference counting for the handle).
Otherwise, LGTM. And apologies for taking this long with the review. If you've already forgotten about this PR, I can take over and work in feedback.
@build-analysisbuild-analysisBot
mentioned this pull request
Feb 2, 2025

Closed

ManickaP
ManickaP
approved these changes
Feb 3, 2025
View reviewed changes
@ManickaPManickaP
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, thank you very much for the contribution and patience.
@ManickaP
ManickaP
merged commit ad382ca
into
dotnet:mainFeb 3, 2025
@pedrobsaila
pedrobsaila
deleted the

83486

branch
February 3, 2025 10:04
grendello added a commit
to grendello/runtime
that referenced
this pull request
Feb 3, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
28eb88e
* main:
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
Keep test PDB in helix payload for native AOT (dotnet#111949)
Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
Always inline number conversions (dotnet#112061)
Use Contains{Any} in Regex source generator (dotnet#112065)
Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
JIT: Update type when return temp is freshly created (dotnet#111948)
Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
WasmAppBuilder: Remove double computation of a value (dotnet#112047)
Disable LTCG for brotli and zlibng. (dotnet#111805)
JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
simplify x86 special intrinsic imports (dotnet#111836)
JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
Fix explicit offset of ByRefLike fields. (dotnet#111584)
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 6, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@ManickaPManickaPManickaP approved these changes

+1 more reviewer

@liveansliveansliveans left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Httpcommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed


5 participants

@pedrobsaila@karelz@CarnaViire@ManickaP@liveans

, '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
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed - #109799
Merged
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Feb 3, 2025
Merged
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed#109799
ManickaP merged 3 commits into
dotnet:mainfrom 
pedrobsaila:83486
Conversation
@pedrobsaila
@pedrobsailapedrobsaila
commented
Nov 13, 2024

Copy link


Copy Markdown

Contributor
Fixes#83486
@dotnet-policy-servicedotnet-policy-serviceBot
added
the 
community-contribution
Indicates that the PR has been added by a community member
label
Nov 13, 2024

This was referenced Nov 14, 2024

Closed


Closed

@CarnaViireCarnaViire
self-assigned this
Nov 14, 2024
liveans
liveans
previously approved these changes
Dec 9, 2024
View reviewed changes
@liveansliveans
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

LGTM
@karelzkarelz
added this to the 10.0.0 milestone
Dec 9, 2024
@liveans
liveans
dismissed
their stale reviewDecember 9, 2024 14:48
Natalia had some concerns, need to deep-dive a bit more
@karelz
karelz
commented
Dec 17, 2024

Copy link


Copy Markdown

Member
@pedrobsaila FYI - People disappeared for the holidays, so we will not be able to get back to it until January. Hopefully it is alright. Sorry for the inconvenience!
@pedrobsaila
pedrobsaila
commented
Dec 17, 2024

Copy link


Copy Markdown

ContributorAuthor
no worries @karelz happy holidays to all the team
@CarnaViire
CarnaViire
commented
Jan 14, 2025

Copy link


Copy Markdown

Member
Sorry for the delay. Now I'm back from vacation, so I will be on it shortly.
@ManickaP
ManickaP
commented
Jan 30, 2025

Copy link


Copy Markdown

Member
There's also another place with DisposeAndClearHandle:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Line 1036
in
99b373a
SafeWinHttpHandle.DisposeAndClearHandle(refconnectHandle);
Could you also replace it with an ordinary dispose - it's on a local variable so not nulling won't affect anything. So that we are consistent in how we dispose handles.
Lastly, can we use Interlocked.CompareExchange in Dispose:
runtime/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines 542 to 544
in
99b373a
if(!_disposed)
{
_disposed=true;


to be 100% sure that we don't call dispose twice on the session handle in some obscure race condition (which could mess up reference counting for the handle).
Otherwise, LGTM. And apologies for taking this long with the review. If you've already forgotten about this PR, I can take over and work in feedback.
@build-analysisbuild-analysisBot
mentioned this pull request
Feb 2, 2025

Closed

ManickaP
ManickaP
approved these changes
Feb 3, 2025
View reviewed changes
@ManickaPManickaP
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, thank you very much for the contribution and patience.
@ManickaP
ManickaP
merged commit ad382ca
into
dotnet:mainFeb 3, 2025
@pedrobsaila
pedrobsaila
deleted the

83486

branch
February 3, 2025 10:04
grendello added a commit
to grendello/runtime
that referenced
this pull request
Feb 3, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
28eb88e
* main:
System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799)
Keep test PDB in helix payload for native AOT (dotnet#111949)
Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054)
Always inline number conversions (dotnet#112061)
Use Contains{Any} in Regex source generator (dotnet#112065)
Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013)
JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590)
Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011)
Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046)
JIT: Compute `fgCalledCount` after synthesis (dotnet#112041)
Simplify boolean logic in `TimeZoneInfo` (dotnet#112062)
JIT: Update type when return temp is freshly created (dotnet#111948)
Remove unused build controls and simplify DotNetBuild.props (dotnet#111986)
Fix case-insensitive JSON deserialization of enum member names (dotnet#112028)
WasmAppBuilder: Remove double computation of a value (dotnet#112047)
Disable LTCG for brotli and zlibng. (dotnet#111805)
JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595)
simplify x86 special intrinsic imports (dotnet#111836)
JIT: Try to retain entry weight during profile synthesis (dotnet#111971)
Fix explicit offset of ByRefLike fields. (dotnet#111584)
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Mar 6, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@ManickaPManickaPManickaP approved these changes

+1 more reviewer

@liveansliveansliveans left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@CarnaViireCarnaViire

Labels

area-System.Net.Httpcommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed


5 participants

@pedrobsaila@karelz@CarnaViire@ManickaP@liveans