supabase-community / postgrest-csharp

A C# Client library for Postgrest
https://supabase-community.github.io/postgrest-csharp/api/Postgrest.html
MIT License
114 stars 22 forks source link

Update Helpers.cs #96

Closed pur3extreme closed 1 month ago

pur3extreme commented 1 month ago
acupofjose commented 1 month ago

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 commented 1 month ago

Thanks @pur3extreme!

pur3extreme commented 1 month ago

@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 commented 1 month ago

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

pur3extreme commented 1 month ago

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

wiverson commented 1 month ago

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