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] WS & HTTP clients more async #95483

Merged
merged 6 commits into from
Dec 2, 2023

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Nov 30, 2023

Motivation

I would like to make dispatch of async JSImport onto different thread automatic inside of the JSImport internals.
Draft here

It will have similar effect for the WS+HTTP clients like this change.
So I'm making the necessary changes ahead of time here.

Changes

  • stop passing null as synchronous singnal in the MT implementation of WS
  • use async SynchronizationContext.Post instead Send where the underlying method is async
  • fix unit tests

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Nov 30, 2023
@pavelsavara pavelsavara added this to the 9.0.0 milestone Nov 30, 2023
@pavelsavara pavelsavara self-assigned this Nov 30, 2023
@ghost
Copy link

ghost commented Nov 30, 2023

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

Issue Details

Motivation

I would like to make dispatch of async JSImport onto different thread automatical inside of the JSImport internals.
It will have similar effect for the WS+HTTP client like this change.
So I'm making the necessary changes ahead of time here.

Changes

  • stop passing null as synchronous singnal in the MT implementation of WS
  • use async SynchronizationContext.Post instead Send where the underlying method is async
  • fix unit tests
Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript, os-browser

Milestone: 9.0.0

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

# Conflicts:
#	src/mono/wasm/runtime/cancelable-promise.ts
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

Unrelated MT failure: Log

[15:37:59] info: [2023-12-02T15:37:59.922Z] [FAIL] System.Text.Json.Serialization.Tests.ReferenceHandlerTestsDynamic_AsyncStreamWithSmallBuffer.TestJsonPathDoesNotFailOnMultiThreads
[15:37:59] info: Assert.Throws() Failure: Exception type was not an exact match
[15:37:59] info: Expected: typeof(System.Text.Json.JsonException)
[15:37:59] info: Actual:   typeof(System.ArgumentException)
[15:37:59] info: ---- System.ArgumentException : An item with the same key has already been added.
[15:37:59] info:    at System.Text.Json.Serialization.Tests.ReferenceHandlerTests.TestIdTask()
[15:37:59] info:    at System.Text.Json.Serialization.Tests.ReferenceHandlerTests.TestJsonPathDoesNotFailOnMultiThreads()
[15:37:59] info: --- End of stack trace from previous location ---
[15:37:59] info: ----- Inner Stack Trace -----
[15:37:59] info:    at System.Runtime.CompilerServices.ConditionalWeakTable`2[[System.Text.Json.JsonSerializerOptions, System.Text.Json, Version=9.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51],[System.Text.Json.JsonSerializerOptions, System.Text.Json, Version=9.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51]].Add(JsonSerializerOptions key, JsonSerializerOptions value)
[15:37:59] info:    at System.Text.Json.Serialization.Tests.JsonSerializerWrapper.JsonSerializerOptionsSmallBufferMapper.ResolveOptionsInstanceWithSmallBuffer(JsonSerializerOptions options)
[15:37:59] info:    at System.Text.Json.Serialization.Tests.JsonSerializerWrapper.AsyncStreamSerializerWrapper.ResolveOptionsInstance(JsonSerializerOptions options)
[15:37:59] info:    at System.Text.Json.Serialization.Tests.JsonSerializerWrapper.AsyncStreamSerializerWrapper.<DeserializeWrapper>d__16`1[[System.Text.Json.Serialization.Tests.ReferenceHandlerTests.Employee, System.Text.Json.Tests, Version=9.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51]].MoveNext()
[15:37:59] info:    at System.Text.Json.Serialization.Tests.StreamingJsonSerializerWrapper.<DeserializeWrapper>d__20`1[[System.Text.Json.Serialization.Tests.ReferenceHandlerTests.Employee, System.Text.Json.Tests, Version=9.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51]].MoveNext()
[15:37:59] info:    at System.Text.Json.Serialization.Tests.ReferenceHandlerTests.<TestIdTask>b__65_0()

@pavelsavara pavelsavara merged commit f90bb60 into dotnet:main Dec 2, 2023
131 of 135 checks passed
@pavelsavara pavelsavara deleted the browser_ws_http_async branch December 2, 2023 17:59
@github-actions github-actions bot locked and limited conversation to collaborators Jan 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants