Skip to content
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

[browser][MT] System.Xml.Tests.TC_SchemaSet_Add_URL.v4 #75123

Closed
simonrozsival opened this issue Sep 6, 2022 · 8 comments · Fixed by #95370
Closed

[browser][MT] System.Xml.Tests.TC_SchemaSet_Add_URL.v4 #75123

simonrozsival opened this issue Sep 6, 2022 · 8 comments · Fixed by #95370
Assignees
Labels
arch-wasm WebAssembly architecture area-VM-threading-mono disabled-test The test is disabled in source code against the issue os-browser Browser variant of arch-wasm
Milestone

Comments

@simonrozsival
Copy link
Member

simonrozsival commented Sep 6, 2022

The test internally calls the XmlDownloadManager which makes a GET HTTP request to http://bla/ and at that point the test execution gets stuck and eventually timeouts. I assume that this is caused by making a HTTP request on a non-browser thread but there might be something else going on.

Ref #74408

Stack:

-> using (Stream respStream = await client.GetStreamAsync(uri).ConfigureAwait(false))
-> return GetNonFileStreamAsync(uri, credentials, proxy).GetAwaiter().GetResult();
-> XmlDownloadManager.GetStream(absoluteUri, _credentials, _proxy);
-> stream = (Stream?)_laterInitParam.inputUriResolver.GetEntity(_laterInitParam.inputbaseUri, string.Empty, typeof(Stream));
-> FinishInitUriString();
-> XmlResolver tmpResolver = GetXmlResolver() ?? GetDefaultPermissiveResolver();
-> XmlReader reader = XmlReader.Create(schemaUri, _readerSettings);
-> sc.Add("xsdauthor", "http://Bla");

GetNonFileStreamAsync is called with a comment:

// This code should be changed if HttpClient ever gets real synchronous methods.  For now,
// we just use the asynchronous methods and block waiting for them to complete.

Expected exception form the hanging function: [out of order message from the browser]: http://bla/ - Failed to load resource: net::ERR_NAME_NOT_RESOLVED

@simonrozsival simonrozsival added arch-wasm WebAssembly architecture area-Build-mono labels Sep 6, 2022
@simonrozsival simonrozsival added this to the 8.0.0 milestone Sep 6, 2022
@ghost
Copy link

ghost commented Sep 6, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

The test internally calls the XmlDownloadManager which makes a GET HTTP request to http://bla/ and at that point the test execution gets stuck and eventually timeouts. I assume that this is caused by making a HTTP request on a non-browser thread but there might be something else going on.

Author: simonrozsival
Assignees: -
Labels:

arch-wasm, area-Build-mono

Milestone: 8.0.0

@lewing
Copy link
Member

lewing commented Sep 6, 2022

@pavelsavara @lambdageek @kg this would be a good first issue for threaded js interop

@kg
Copy link
Member

kg commented Sep 6, 2022

I wonder if unhandled exceptions in workers get forwarded to the console during tests?

@pavelsavara
Copy link
Member

I wonder if unhanded exceptions in workers get forwarded to the console during tests?

We improved it here #74751

@lewing lewing modified the milestones: 8.0.0, 9.0.0 Jul 24, 2023
@pavelsavara pavelsavara self-assigned this Nov 8, 2023
@pavelsavara pavelsavara changed the title [wasm-mt] The System.Xml.Tests.TC_SchemaSet_Add_URL.v4 test gets stuck on Browser with multi-threading enabled [browser][MT] System.Xml.Tests.TC_SchemaSet_Add_URL.v4 Nov 9, 2023
@pavelsavara
Copy link
Member

@ilonatommy this is possibly working now, could you please test/enable ?

@pavelsavara pavelsavara added the os-browser Browser variant of arch-wasm label Nov 9, 2023
@ilonatommy
Copy link
Member

It still hangs but ST works, I am changing the blocking condition in #94558

@ilonatommy
Copy link
Member

ilonatommy commented Nov 23, 2023

The problem is this operation: .GetAwaiter().GetResult(), not HTTP request alone. When we make a HTTP request directly from the test (converted to async), it passes with the expected exception.

public async Task v4()
{
     try
     {
         var handler = new HttpClientHandler();
         string uri = "http://Bla";
         using (var client = new HttpClient(handler))
         {
              using (Stream respStream = await client.GetStreamAsync("http://Bla").ConfigureAwait(false)){} // Exception, correct!
         }
         XmlSchemaSet sc = new XmlSchemaSet();
         sc.Add("xsdauthor", "http://Bla");
     }
...

However, when we wrap the HTTP request in a helper async function and we use block-waiting for that function in the sync test, it hangs.

public void v4()
{
     try
     {
         AsyncHttpWrapper().GetAwaiter().GetResult(); // hangs
         XmlSchemaSet sc = new XmlSchemaSet();
         sc.Add("xsdauthor", "http://Bla");
     }
....

Theoretically, we could not use GetStream in MT but GetStreamAsync, I will see if that helps (it avoids block-waiting).

Edit: no, it does not help because eventually, in FinishInitUriString we are blocking the async calls anyway stream = (Stream)t.GetAwaiter().GetResult();

@pavelsavara
Copy link
Member

I think GetResult() will (spin) block-wait the thread and the same threads needs event loop to resolve HTTP promise. This is nasty.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 17, 2024
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 21, 2024
@github-project-automation github-project-automation bot moved this to Done in wasm-mt Aug 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-VM-threading-mono disabled-test The test is disabled in source code against the issue os-browser Browser variant of arch-wasm
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants