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

[SignalR] Avoid initial memory allocation in ChannelBasedSemaphore #47588

Open
anobaka opened this issue Apr 6, 2023 · 0 comments
Open

[SignalR] Avoid initial memory allocation in ChannelBasedSemaphore #47588

anobaka opened this issue Apr 6, 2023 · 0 comments
Labels
area-signalr Includes: SignalR clients and servers design-proposal This issue represents a design proposal for a different issue, linked in the description

Comments

@anobaka
Copy link

anobaka commented Apr 6, 2023

Summary

The initial memory allocation should be avoided in ChannelBasedSemaphore

Motivation and goals

Some developers could just give HubConnectionContextOptions.MaximumParallelInvocations a large value (such as int.MaxValue) to ensure their single client can make as many parallel invocations as possible.

Unfortunately , ChannelBasedSemaphore will always allocate 'enough' memory according to HubConnectionContextOptions.MaximumParallelInvocations on first invocation, even if there is only one invocation here, and the whole program may crash or have high cpu and memory usage without a friendly warning. This behavior increases the costs of diagnostics.

This is a sample message for an event about this error in Event Viewer in Win10

Faulting application name: SignalRTest.exe, version: 1.0.0.0, time stamp: 0x634eda42
Faulting module name: coreclr.dll, version: 7.0.22.51805, time stamp: 0x634ed04e
Exception code: 0xc0000005
Fault offset: 0x000000000013b93e
Faulting process id: 0x820c
Faulting application start time: 0x01d9682ee7ded2d7
Faulting application path: C:\Users\anoba\source\repos\SignalRTest\SignalRTest\bin\Debug\net7.0\SignalRTest.exe
Faulting module path: C:\Program Files\dotnet\shared\Microsoft.NETCore.App\7.0.0\coreclr.dll
Report Id: a686bc6b-6375-4553-b5b3-0b26dc5403dc
Faulting package full name: 
Faulting package-relative application ID: 

By the way, will this cause an inconsistent result for available invocation quotas? I didn't see any Release corresponding the result of TryAcquire.

In scope

This is my immature idea. Maybe we can use the Interlocked to implement TryAcquire, and use the combination of Interlocked and Channel for WaitAsync and RunAsync ?

@anobaka anobaka added the design-proposal This issue represents a design proposal for a different issue, linked in the description label Apr 6, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-signalr Includes: SignalR clients and servers label Apr 6, 2023
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers design-proposal This issue represents a design proposal for a different issue, linked in the description
Projects
None yet
Development

No branches or pull requests

2 participants