Skip to content

Commit 639e4f4

Browse files
authored
IEP-1584 Avoid Simultaneous Modals on Old Config Detection (#1264)
fix: added global modal lock to avoid multiple modals
1 parent 30b766a commit 639e4f4

File tree

4 files changed

+159
-124
lines changed

4 files changed

+159
-124
lines changed
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*******************************************************************************
2+
* Copyright 2025 Espressif Systems (Shanghai) PTE LTD. All rights reserved.
3+
* Use is subject to license terms.
4+
*******************************************************************************/
5+
package com.espressif.idf.ui;
6+
7+
import java.util.concurrent.Semaphore;
8+
import java.util.function.Consumer;
9+
import java.util.function.Supplier;
10+
11+
import org.eclipse.swt.widgets.Display;
12+
13+
public class GlobalModalLock
14+
{
15+
private static final Semaphore lock = new Semaphore(1);
16+
17+
private GlobalModalLock()
18+
{
19+
}
20+
21+
public static <T> void showModal(Supplier<T> dialogSupplier, Consumer<T> callback)
22+
{
23+
new Thread(() -> {
24+
try
25+
{
26+
lock.acquire();
27+
Display.getDefault().syncExec(() -> {
28+
try
29+
{
30+
T result = dialogSupplier.get();
31+
callback.accept(result);
32+
} finally
33+
{
34+
lock.release();
35+
}
36+
});
37+
}
38+
catch (InterruptedException e)
39+
{
40+
Thread.currentThread().interrupt();
41+
}
42+
}, "GlobalModalLock-DialogThread").start(); //$NON-NLS-1$
43+
}
44+
}

bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/LaunchBarListener.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import org.eclipse.core.runtime.IProgressMonitor;
2020
import org.eclipse.core.runtime.NullProgressMonitor;
2121
import org.eclipse.debug.core.DebugPlugin;
22-
import org.eclipse.core.runtime.Status;
23-
import org.eclipse.core.runtime.jobs.Job;
2422
import org.eclipse.debug.core.ILaunchConfiguration;
2523
import org.eclipse.debug.core.ILaunchManager;
2624
import org.eclipse.debug.core.ILaunchMode;
@@ -146,15 +144,17 @@ private void update(String newTarget)
146144
// If both are not same
147145
if (currentTarget != null && !newTarget.equals(currentTarget))
148146
{
149-
150-
boolean isDelete = MessageDialog.openQuestion(EclipseUtil.getShell(),
147+
GlobalModalLock.showModal(() -> MessageDialog.openQuestion(EclipseUtil.getShell(),
151148
Messages.LaunchBarListener_TargetChanged_Title,
152149
MessageFormat.format(Messages.LaunchBarListener_TargetChanged_Msg,
153-
project.getName(), currentTarget, newTarget));
154-
if (isDelete)
155-
{
156-
deleteBuildFolder(project, buildLocation);
157-
}
150+
project.getName(), currentTarget, newTarget)),
151+
isDelete -> {
152+
if (isDelete)
153+
{
154+
deleteBuildFolder(project, buildLocation);
155+
156+
}
157+
});
158158
}
159159
}
160160
}

bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/EspressifToolStartup.java

Lines changed: 94 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.espressif.idf.core.tools.watcher.EimJsonWatchService;
4242
import com.espressif.idf.core.util.IDFUtil;
4343
import com.espressif.idf.core.util.StringUtil;
44+
import com.espressif.idf.ui.GlobalModalLock;
4445
import com.espressif.idf.ui.IDFConsole;
4546
import com.espressif.idf.ui.UIPlugin;
4647
import com.espressif.idf.ui.handlers.EclipseHandler;
@@ -68,14 +69,13 @@ public class EspressifToolStartup implements IStartup
6869
@Override
6970
public void earlyStartup()
7071
{
71-
preferences = org.eclipse.core.runtime.preferences.InstanceScope.INSTANCE
72-
.getNode(UIPlugin.PLUGIN_ID);
72+
preferences = org.eclipse.core.runtime.preferences.InstanceScope.INSTANCE.getNode(UIPlugin.PLUGIN_ID);
7373
toolInitializer = new ToolInitializer(preferences);
7474
standardConsoleStream = getConsoleStream(false);
7575
errorConsoleStream = getConsoleStream(true);
7676
idfEnvironmentVariables = new IDFEnvironmentVariables();
77-
eimLoader = new EimLoader(new StartupClassDownloadEimDownloadListener(),
78-
standardConsoleStream, errorConsoleStream, Display.getDefault());
77+
eimLoader = new EimLoader(new StartupClassDownloadEimDownloadListener(), standardConsoleStream,
78+
errorConsoleStream, Display.getDefault());
7979
EimJsonStateChecker stateChecker = new EimJsonStateChecker(preferences);
8080
eimJsonUiChangeHandler = new EimJsonUiChangeHandler(preferences);
8181
stateChecker.updateLastSeenTimestamp();
@@ -87,7 +87,7 @@ public void earlyStartup()
8787
notifyMissingTools();
8888
return;
8989
}
90-
90+
9191
eimJson = toolInitializer.loadEimJson();
9292

9393
if (toolInitializer.isOldEspIdfConfigPresent() && !toolInitializer.isOldConfigExported())
@@ -98,8 +98,8 @@ public void earlyStartup()
9898
{
9999
promptUserToMoveEimToApplications();
100100
}
101-
102-
EimJsonWatchService.withPausedListeners(()-> handleOldConfigExport());
101+
102+
EimJsonWatchService.withPausedListeners(() -> handleOldConfigExport());
103103
}
104104
else if (toolInitializer.isEimIdfJsonPresent() && !toolInitializer.isEspIdfSet())
105105
{
@@ -111,7 +111,7 @@ else if (toolInitializer.isEimIdfJsonPresent() && !toolInitializer.isEspIdfSet()
111111
{
112112
idfEnvironmentVariables.addEnvVariable(IDFEnvironmentVariables.EIM_PATH, eimJson.getEimPath());
113113
}
114-
else
114+
else
115115
{
116116
// Fail-safe call to ensure if the eim is in Applications or user.home it is setup in env vars
117117
toolInitializer.findAndSetEimPath();
@@ -127,113 +127,113 @@ private boolean checkIfEimPathMacOsIsInApplications()
127127
{
128128
if (!Platform.getOS().equals(Platform.OS_MACOSX))
129129
return true;
130-
131-
String eimPath = idfEnvironmentVariables.getEnvValue(IDFEnvironmentVariables.EIM_PATH);
130+
131+
String eimPath = idfEnvironmentVariables.getEnvValue(IDFEnvironmentVariables.EIM_PATH);
132132
if (!StringUtil.isEmpty(eimPath))
133133
{
134134
if (Files.exists(Paths.get(eimPath)))
135135
{
136-
boolean isInApplications = eimPath.startsWith("/Applications/") ||
137-
eimPath.startsWith(System.getProperty("user.home") + "/Applications/");
136+
boolean isInApplications = eimPath.startsWith("/Applications/")
137+
|| eimPath.startsWith(System.getProperty("user.home") + "/Applications/");
138138
if (!isInApplications)
139139
{
140140
Logger.log("EIM_PATH not in applications: " + eimPath);
141141
return false;
142142
}
143143
}
144144
}
145-
145+
146146
return true;
147147
}
148148

149149
private void handleOldConfigExport()
150150
{
151-
final int[] response = new int[] { -1 };
152151
Display display = Display.getDefault();
153-
display.syncExec(() -> {
154-
MessageDialog messageDialog = new MessageDialog(display.getActiveShell(),
155-
Messages.OldConfigFoundMsgBoxTitle, null, Messages.OldConfigFoundMsgBoxMsg, 0, 0,
156-
new String[] { Messages.ToolsInitializationDifferentPathMessageBoxOptionYes,
157-
Messages.ToolsInitializationDifferentPathMessageBoxOptionNo });
158-
response[0] = messageDialog.open();
159-
});
160152

161-
if (response[0] == 0)
153+
GlobalModalLock.showModal(() -> MessageDialog.openQuestion(display.getActiveShell(),
154+
Messages.OldConfigFoundMsgBoxTitle, Messages.OldConfigFoundMsgBoxMsg), response -> {
155+
if (response)
156+
{
157+
startExportOldConfig();
158+
}
159+
});
160+
161+
}
162+
163+
private void startExportOldConfig()
164+
{
165+
try
162166
{
163-
try
167+
// if eim json is present it means that it contains the updated path and we use that else we fallback to
168+
// finding eim in default paths
169+
Path eimPath;
170+
String eimPathEnvVar = idfEnvironmentVariables.getEnvValue(IDFEnvironmentVariables.EIM_PATH);
171+
if (eimJson != null)
164172
{
165-
// if eim json is present it means that it contains the updated path and we use that else we fallback to finding eim in default paths
166-
Path eimPath;
167-
String eimPathEnvVar = idfEnvironmentVariables.getEnvValue(IDFEnvironmentVariables.EIM_PATH);
168-
if (eimJson != null)
169-
{
170-
eimPath = Paths.get(eimJson.getEimPath());
171-
}
172-
else if (!StringUtil.isEmpty(eimPathEnvVar))
173-
{
174-
eimPath = Paths.get(eimPathEnvVar);
175-
}
176-
else
177-
{
178-
eimPath = toolInitializer.getDefaultEimPath();
179-
}
180-
181-
IStatus status = toolInitializer.exportOldConfig(eimPath);
182-
Logger.log("Tools Conversion Process Message: ");
183-
Logger.log(status.getMessage());
184-
if (status.getSeverity() != IStatus.ERROR)
185-
{
186-
preferences.putBoolean(EimConstants.OLD_CONFIG_EXPORTED_FLAG, true);
187-
displayInformationMessageBox(Messages.OldConfigExportCompleteSuccessMsgTitle,
188-
Messages.OldConfigExportCompleteSuccessMsg);
189-
}
190-
else
191-
{
192-
displayInformationMessageBox(Messages.OldConfigExportCompleteFailMsgTitle,
193-
Messages.OldConfigExportCompleteFailMsg);
194-
}
173+
eimPath = Paths.get(eimJson.getEimPath());
174+
}
175+
else if (!StringUtil.isEmpty(eimPathEnvVar))
176+
{
177+
eimPath = Paths.get(eimPathEnvVar);
178+
}
179+
else
180+
{
181+
eimPath = toolInitializer.getDefaultEimPath();
182+
}
183+
184+
IStatus status = toolInitializer.exportOldConfig(eimPath);
185+
Logger.log("Tools Conversion Process Message: ");
186+
Logger.log(status.getMessage());
187+
if (status.getSeverity() != IStatus.ERROR)
188+
{
189+
preferences.putBoolean(EimConstants.OLD_CONFIG_EXPORTED_FLAG, true);
190+
displayInformationMessageBox(Messages.OldConfigExportCompleteSuccessMsgTitle,
191+
Messages.OldConfigExportCompleteSuccessMsg);
195192
}
196-
catch (IOException e)
193+
else
197194
{
198-
Logger.log("Error exporting old configuration", e);
199195
displayInformationMessageBox(Messages.OldConfigExportCompleteFailMsgTitle,
200196
Messages.OldConfigExportCompleteFailMsg);
201197
}
202198
}
199+
catch (IOException e)
200+
{
201+
Logger.log("Error exporting old configuration", e);
202+
displayInformationMessageBox(Messages.OldConfigExportCompleteFailMsgTitle,
203+
Messages.OldConfigExportCompleteFailMsg);
204+
}
203205
}
204206

205207
private void displayInformationMessageBox(String messageTitle, String message)
206208
{
207209
Display display = Display.getDefault();
208-
display.syncExec(() -> {
210+
GlobalModalLock.showModal(() -> {
209211
MessageDialog.openInformation(display.getActiveShell(), messageTitle, message);
212+
return null;
213+
}, ignored -> {
210214
});
211215
}
212216

213217
private void showEimJsonStateChangeNotification()
214218
{
215-
int response = eimJsonUiChangeHandler.displayMessageToUser();
216-
eimJsonUiChangeHandler.handleUserResponse(response);
219+
eimJsonUiChangeHandler.displayMessageToUser();
217220
}
218221

219222
private void notifyMissingTools()
220223
{
221-
boolean [] userAgreed = new boolean[1];
222-
Display.getDefault().syncExec(() -> {
223-
userAgreed[0] = MessageDialog.openQuestion(Display.getDefault().getActiveShell(),
224-
Messages.ToolsInitializationEimMissingMsgBoxTitle,
225-
Messages.ToolsInitializationEimMissingMsgBoxMessage);
226-
});
227-
228-
if (userAgreed[0])
229-
{
230-
// Download Launch EIM
231-
downloadAndLaunchEim();
232-
}
233-
else
234-
{
235-
Logger.log("User selected No to download EIM");
236-
}
224+
GlobalModalLock.showModal(() -> MessageDialog.openQuestion(Display.getDefault().getActiveShell(),
225+
Messages.ToolsInitializationEimMissingMsgBoxTitle, Messages.ToolsInitializationEimMissingMsgBoxMessage),
226+
response -> {
227+
if (response)
228+
{
229+
// Download Launch EIM
230+
downloadAndLaunchEim();
231+
}
232+
else
233+
{
234+
Logger.log("User selected No to download EIM");
235+
}
236+
});
237237
}
238238

239239
private void downloadAndLaunchEim()
@@ -295,19 +295,22 @@ private void promptUserToOpenToolManager(EimJson eimJson)
295295
messageBox.setText(Messages.NoActiveEspIdfInWorkspaceMsgTitle);
296296
messageBox.setMessage(Messages.NoActiveEspIdfInWorkspaceMsg);
297297

298-
if (messageBox.open() == SWT.YES)
299-
{
300-
openEspIdfManager(eimJson);
301-
}
298+
GlobalModalLock.showModal(messageBox::open, response -> {
299+
if (response == SWT.YES)
300+
{
301+
openEspIdfManager(eimJson);
302+
}
303+
});
302304
});
303305
}
304-
306+
305307
private void promptUserToMoveEimToApplications()
306308
{
307-
Display.getDefault().asyncExec(() -> {
308-
MessageDialog.openInformation(
309-
Display.getDefault().getActiveShell(),
310-
Messages.EIMNotInApplicationsTitle, Messages.EIMNotInApplicationsMessage);
309+
GlobalModalLock.showModal(() -> {
310+
MessageDialog.openInformation(Display.getDefault().getActiveShell(), Messages.EIMNotInApplicationsTitle,
311+
Messages.EIMNotInApplicationsMessage);
312+
return null;
313+
}, ignored -> {
311314
});
312315
}
313316

@@ -328,7 +331,7 @@ private void openEspIdfManager(EimJson eimJson)
328331
}
329332
});
330333
}
331-
334+
332335
private class StartupClassDownloadEimDownloadListener implements DownloadListener
333336
{
334337

@@ -347,7 +350,7 @@ public void onProgress(int percent)
347350
Logger.log(e);
348351
}
349352
});
350-
353+
351354
}
352355

353356
@Override
@@ -363,7 +366,7 @@ public void onCompleted(String filePath)
363366
Logger.log(e);
364367
}
365368
});
366-
369+
367370
Process process = null;
368371
String appToLaunch = filePath;
369372
try
@@ -372,15 +375,17 @@ public void onCompleted(String filePath)
372375
{
373376
appToLaunch = eimLoader.installAndLaunchDmg(Paths.get(filePath));
374377
}
375-
378+
376379
idfEnvironmentVariables.addEnvVariable(IDFEnvironmentVariables.EIM_PATH, appToLaunch);
377380
process = eimLoader.launchEim(appToLaunch);
378381
}
379-
catch (IOException | InterruptedException e)
382+
catch (
383+
IOException
384+
| InterruptedException e)
380385
{
381386
Logger.log(e);
382387
}
383-
388+
384389
eimLoader.waitForEimClosure(process, () -> {
385390
if (toolInitializer.isOldEspIdfConfigPresent() && !toolInitializer.isOldConfigExported())
386391
{

0 commit comments

Comments
 (0)