-
Notifications
You must be signed in to change notification settings - Fork 41
Fix deadlock when showing JML warnings dialog #3692
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
Fix deadlock when showing JML warnings dialog #3692
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3692 +/- ##
=========================================
Coverage 47.99% 47.99%
Complexity 16044 16044
=========================================
Files 1683 1683
Lines 96044 96044
Branches 15387 15387
=========================================
Hits 46093 46093
Misses 44681 44681
Partials 5270 5270 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I would say, that this is not the patch we would want in just a case. Could you provide a stack trace showing the location from which the method is invoked? |
|
Sure, here you go: |
|
I would say the responsibility for EDT is in diff --git a/key.ui/src/main/java/de/uka/ilkd/key/gui/WindowUserInterfaceControl.java b/key.ui/src/main/java/de/uka/ilkd/key/gui/WindowUserInterfaceControl.java
index 8f738947e8..5ab4910923 100644
--- a/key.ui/src/main/java/de/uka/ilkd/key/gui/WindowUserInterfaceControl.java
+++ b/key.ui/src/main/java/de/uka/ilkd/key/gui/WindowUserInterfaceControl.java
@@ -592,7 +592,9 @@ public class WindowUserInterfaceControl extends AbstractMediatorUserInterfaceCon
@Override
public void reportWarnings(ImmutableSet<PositionedString> warnings) {
- IssueDialog.showWarningsIfNecessary(mainWindow, warnings);
+ SwingUtilities.invokeLater(() ->
+ IssueDialog.showWarningsIfNecessary(mainWindow, warnings)
+ );
}
/** |
ce417f5 to
5e877bd
Compare
5e877bd to
1c76f67
Compare
|
Now the warnings dialog will be shown in parallel to the contract chooser, which is a bit odd. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the warnings dialog will be shown in parallel to the contract chooser, which is a bit odd.
In our meeting, we agreed that this is very difficult to prevent (probably involves writing a class that has an explicit queue of dialogs to show), so we decided to ignore that for the moment.
Thanks, @FliegendeWurst and @wadoon.
Review was supposed to accept, not to request change ;-)
WolframPfeifer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the warnings dialog will be shown in parallel to the contract chooser, which is a bit odd.
In our meeting, we decided to ignore that shortcoming, since a fix would be difficult (probably requires writing a class that keeps an explicit queue of dialogs to show).
Thanks, @FliegendeWurst and @wadoon!
Intended Change
It seems this method is called on the worker thread, which may lead to a deadlock when creating the dialog.
Type of pull request
Ensuring quality
The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.