Skip to content

Fix deadlock in mysql poolmanager #60

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

Closed

Conversation

brokuene
Copy link

There is deadlock bug in MySqlPoolManager.cs. It makes the library unusable in any .Net applications with SynchronizationCotnext while using pooling feature.

It causes a deadlock of threads, application will hang forever. Affected are:

  • ASP .NET MVC app (on full .net framework),
  • Windows Forms App,
  • WPF app.
    If using Pooling=true in connection string (common scenario).

What is more, it leads to breaking the whole appPool on IIS (for ASP .NET MVC app) as all threads from thread pool will eventually deadlock.

This looks like obvious human error, there is missing ConfigureAwait(false) when awaiting on semaphore. Note that the ConfigureAwait(false) is widely used in all other places at this class but in this one is simply missing.

await waitHandle.WaitAsync(CancellationToken.None);

In this PR:

  • Two places in MySqlPoolManager.cs are fixed to not cause deadlock
  • Changelog and library version updated (I hope this critical bugfix will reach all users sooner than later)
  • Unit test which allows to reproduce the issue (for the sake of simplicity I included only one of the SynchronizationContext scenario to demonstrate deadlock).

How bug was introduced

  1. On 11 Jan 2023 there was change from using lock to SemaphoreSlim in MySqlPoolManager.GetPoolAsync. Unfortunatelly this introduced a bug - the new Semaphore was created every time method is invoked so there was no threadsaffenes at all.
  2. The bug from 1. was fixed by setting the semaphore static so the thread safeness was introduced again. Unfortunately acquiring the lock on semaphore is done asynchronously without using ConfigureAwait(false) so it causes deadlocks in described in this PR scenarios.

…eAwait(false). That affects all apps with SynchronizationContext for example: ASP.NET MVC fullframework app, Windows Forms app, WPF app.
@mysql-oca-bot
Copy link

Hi, thank you for submitting this pull request. In order to consider your code we need you to sign the Oracle Contribution Agreement (OCA). Please review the details and follow the instructions at https://oca.opensource.oracle.com/
Please make sure to include your MySQL bug system user (email) in the returned form.
Thanks

@brokuene
Copy link
Author

@mysql-admin My OCA is under review

@mysql-oca-bot
Copy link

Hi, there was no response to our request to sign an OCA or confirm the code is submitted under the terms of the OCA. As such this request will be closed.
Thanks

@h-singhal
Copy link

@mysql-oca-bot @mysql-admin We are currently facing the issue mentioned. The fix is missing in version 8.3.0, so we are forced to downgrade to 8.0.28.

@brokuene
Copy link
Author

@h-singhal

We also had to downgrade, to 8.0.32 - this is the highest, I believe, with no huge flaws, but maybe you know something more, why 8.0.28 not 8.0.32 ?

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