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

Fixed deadlock in class initialization. #10540

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Sep 18, 2023

  • Thread T1 may initialize HttpTester.Message that extends MutableHttpFields, so grabs the lock for the initialization of class MutableHttpFields.
  • Thread T2 may initialize HttpFields, so grabs the lock for HttpFields and initializes field EMPTY, which calls new MutableHttpFields.
  • To initialize MutableHttpFields, T1 must initialize HttpFields, but sees that its lock is taken and waits.
  • To initialize HttpFields, T2 must create an instance and therefore initialize MutableHttpFields, but sees that its lock is taken and waits.
  • Deadlock.

The solution is to use another class, EmptyHttpFields, to initialize HttpFields.EMPTY, so that there is no deadlock.

* Thread T1 may initialize HttpTester.Message that extends MutableHttpFields, so grabs the lock for the initialization of class MutableHttpFields.
* Thread T2 may initialize HttpFields, so grabs the lock for HttpFields and initializes field EMPTY, which calls new MutableHttpFields.
* To initialize MutableHttpFields, T1 must initialize HttpFields, but sees that its lock is taken and waits.
* To initialize HttpFields, T2 must create an instance and therefore initialize MutableHttpFields, but sees that its lock is taken and waits.
* Deadlock.

The solution is to use another class, EmptyHttpFields, to initialize HttpFields.EMPTY, so that there is no deadlock.

Signed-off-by: Simone Bordet <[email protected]>
@sbordet sbordet requested a review from gregw September 18, 2023 07:02
@gregw gregw merged commit 3222757 into jetty-12.0.x Sep 18, 2023
@gregw gregw deleted the fix/jetty-12-httpfields-deadlock branch September 18, 2023 07:37
@joakime
Copy link
Contributor

joakime commented Sep 18, 2023

Do we want to merge this change back to Jetty 10/11 too?

@sbordet
Copy link
Contributor Author

sbordet commented Sep 18, 2023

@joakime yes it should. Can you please do it?

joakime pushed a commit that referenced this pull request Sep 18, 2023
* Thread T1 may initialize HttpTester.Message that extends MutableHttpFields, so grabs the lock for the initialization of class MutableHttpFields.
* Thread T2 may initialize HttpFields, so grabs the lock for HttpFields and initializes field EMPTY, which calls new MutableHttpFields.
* To initialize MutableHttpFields, T1 must initialize HttpFields, but sees that its lock is taken and waits.
* To initialize HttpFields, T2 must create an instance and therefore initialize MutableHttpFields, but sees that its lock is taken and waits.
* Deadlock.

The solution is to use another class, EmptyHttpFields, to initialize HttpFields.EMPTY, so that there is no deadlock.

Signed-off-by: Simone Bordet <[email protected]>
joakime added a commit that referenced this pull request Sep 18, 2023
…ion. (#10545)

* Fixed deadlock in class initialization. (#10540)

* Thread T1 may initialize HttpTester.Message that extends MutableHttpFields, so grabs the lock for the initialization of class MutableHttpFields.
* Thread T2 may initialize HttpFields, so grabs the lock for HttpFields and initializes field EMPTY, which calls new MutableHttpFields.
* To initialize MutableHttpFields, T1 must initialize HttpFields, but sees that its lock is taken and waits.
* To initialize HttpFields, T2 must create an instance and therefore initialize MutableHttpFields, but sees that its lock is taken and waits.
* Deadlock.

The solution is to use another class, EmptyHttpFields, to initialize HttpFields.EMPTY, so that there is no deadlock.

Signed-off-by: Joakim Erdfelt <[email protected]>
Co-authored-by: Simone Bordet <[email protected]>
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