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

Update Helpers.cs #96

Merged
merged 1 commit into from
May 16, 2024
Merged

Conversation

pur3extreme
Copy link
Contributor

  • Dispose of the response
  • ConfigureAwait(false) the response so it won't have a chance to deadlock the application

- Dispose of the response
- ConfigureAwait(false) the response so it won't have a chance to deadlock the application
@acupofjose
Copy link
Collaborator

Interesting! I'm assuming you're seeings some deadlocks in your application code @pur3extreme?

Was doing a little research and came across this microsoft devblog:

In contrast, general-purpose libraries are “general purpose” in part because they don’t care about the environment in which they’re used. You can use them from a web app or from a client app or from a test, it doesn’t matter, as the library code is agnostic to the app model it might be used in. Being agnostic then also means that it’s not going to be doing anything that needs to interact with the app model in a particular way, e.g. it won’t be accessing UI controls, because a general-purpose library knows nothing about UI controls. Since we then don’t need to be running the code in any particular environment, we can avoid forcing continuations/callbacks back to the original context, and we do that by using ConfigureAwait(false) and gaining both the performance and reliability benefits it brings. This leads to the general guidance of: if you’re writing general-purpose library code, use ConfigureAwait(false). This is why, for example, you’ll see every (or almost every) await in the .NET Core runtime libraries using ConfigureAwait(false) on every await; with a few exceptions, in cases where it doesn’t it’s very likely a bug to be fixed. For example, this PR fixed a missing ConfigureAwait(false) call in HttpClient.

As with all guidance, of course, there can be exceptions, places where it doesn’t make sense. For example, one of the larger exemptions (or at least categories that requires thought) in general-purpose libraries is when those libraries have APIs that take delegates to be invoked. In such cases, the caller of the library is passing potentially app-level code to be invoked by the library, which then effectively renders those “general purpose” assumptions of the library moot. Consider, for example, an asynchronous version of LINQ’s Where method, e.g. public static async IAsyncEnumerable WhereAsync(this IAsyncEnumerable source, Func<T, bool> predicate). Does predicate here need to be invoked back on the original SynchronizationContext of the caller? That’s up to the implementation of WhereAsync to decide, and it’s a reason it may choose not to use ConfigureAwait(false).

Even with these special cases, the general guidance stands and is a very good starting point: use ConfigureAwait(false) if you’re writing general-purpose library / app-model-agnostic code, and otherwise don’t.

Looks like this is a little change that should be made on practically all of the C# libraries...

@acupofjose acupofjose merged commit 228cd7b into supabase-community:master May 16, 2024
1 check passed
@acupofjose
Copy link
Collaborator

Thanks @pur3extreme!

@pur3extreme
Copy link
Contributor Author

@acupofjose I had recently moved over to Supabase from a previously synchronous backend - This a Unity application with around 50 concurrent users after about 24-48 hours of operation the server would deadlock.

Yes, ConfigureAwait(false) should be used where possible in every library - Postgress is the only Supabase library that I really use so its the only one I looked into

@wiverson
Copy link
Collaborator

@pur3extreme just to clarify, are you running Supabase inside of a Unity build that's acting as a server app?

@pur3extreme
Copy link
Contributor Author

@wiverson Yes, its a game server, so sharing the same code base as the client is extremely convenient.

@wiverson
Copy link
Collaborator

Ah. Are you using the stateful client on both the Unity game client and the Unity server? FWIW you might want to focus on the stateless API calls for the server-side, basically the config as described at https://github.com/supabase-community/supabase-csharp/wiki/Server-Side-Applications

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants