Skip to content

Fix SpaServices extensions hangs (#17277) #21819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2020
Merged

Fix SpaServices extensions hangs (#17277) #21819

merged 1 commit into from
May 14, 2020

Conversation

artar94
Copy link
Contributor

@artar94 artar94 commented May 14, 2020

Fixed adding a string with a large number of trailing zeros to StringBuilder, which sometimes caused the thread to hang.

Summary of the changes

  • Only a significant portion of the buffer will be added.

Addresses #17277

Fixed adding a string with a large number of trailing zeros to StringBuilder, which sometimes caused the thread to hang
@artar94 artar94 requested a review from Tratcher as a code owner May 14, 2020 10:40
@ghost ghost added the area-middleware label May 14, 2020
@Tratcher Tratcher requested a review from SteveSandersonMS May 14, 2020 16:02
Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

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

This code would be clearer if re-written using Spans and Slicing.

@Tratcher Tratcher merged commit 17c90a7 into dotnet:master May 14, 2020
@Tratcher Tratcher added this to the 5.0.0-preview6 milestone May 14, 2020
@artar94 artar94 deleted the fix-spa-services-hangs branch May 14, 2020 17:49
@Rinsen
Copy link

Rinsen commented May 15, 2020

@Tratcher Will this be merged to next 3.1.x release also?

@Tratcher
Copy link
Member

Not automatically. Fixes in 3.1 have to meet a high bar. While there was a clear bug here, we'll need people to verify this really fixes the hang they were experiencing. E.g. reproducing the issue in 5.0.0-preview4, but not in a nightly build or preview after this fix (preview 6?).

@Rinsen
Copy link

Rinsen commented May 15, 2020

That I 100% understand.

I tried to brute force the change in on a branch on my own under the 3.1 tag but that ended up in a mess of broken builds so it was kind of a dead end to verify that this was compleatly fixing the issue for us...

What is the easies way to know what preview this will be included in? Will it be in some list or is it just to check when the preview was taken via some tag or branch?

@Tratcher
Copy link
Member

From our current schedule it should go into preview6, but that's a ways out. Do you have a reliable repro using the current preview3? If so we can find you a nightly build with the changes to test.

@Rinsen
Copy link

Rinsen commented May 15, 2020

@Tratcher I can't reproduce this on Microsoft.AspNetCore.SpaServices.Extensions" Version="5.0.0-preview.3.20215.14"

So on .NET 5 it seem to be working just fine with preview 3 already.

If I go back to 3.1.3 I directly have the issue. So something else must have changed related to this so we never end up in the breaking case that this is fixing, I will try to investigate 3.1.3 a bit more and see what is going on.

Edit:
The source code I have been using
https://github.com/Rinsen/Angular9IssueWithSpaServicesExtensions

It's only a file new project on the Angular Template and an updated Angular application to Angular 9 and nothing else.

@amcasey amcasey added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares label Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants