Skip to content

Commit a5c72eb

Browse files
iloveeclipseiloveeclipse
committed
Fix for regression in OpenWorkspaceAction
Code introduced in fa50c5b in `OpenWorkspaceAction.canRestartWithWorkspace(String)` missed the most important part about workspace lock checks - whether the lock is *actually* held by any process. Fixes that and added tests for `WorkspaceLock`. See #3518
1 parent 0b514c7 commit a5c72eb

4 files changed

Lines changed: 230 additions & 7 deletions

File tree

‎bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/actions/OpenWorkspaceAction.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package org.eclipse.ui.internal.ide.actions;
1515

1616
import java.net.MalformedURLException;
17+
import java.net.URL;
1718
import java.nio.file.Path;
1819
import java.util.ArrayList;
1920

@@ -240,10 +241,16 @@ public void restart(String workspacePath) {
240241
private boolean canRestartWithWorkspace(String workspacePath) throws IllegalStateException {
241242
Path selectedWorkspace = Path.of(workspacePath);
242243
try {
243-
String workspaceLockDetails = WorkspaceLock.getWorkspaceLockDetails(selectedWorkspace.toUri().toURL());
244-
if (workspaceLockDetails == null) {
244+
URL url = selectedWorkspace.toUri().toURL();
245+
if (!WorkspaceLock.isWorkspaceLocked(url)) {
245246
return true;
246247
}
248+
String workspaceLockDetails = WorkspaceLock.getWorkspaceLockDetails(url);
249+
if (workspaceLockDetails == null) {
250+
// can only happen if the workspace is locked by an older Eclipse
251+
// which doesn't write lock details
252+
workspaceLockDetails = ""; //$NON-NLS-1$
253+
}
247254
WorkspaceLock.showWorkspaceLockedDialog(window.getShell(), workspacePath, workspaceLockDetails);
248255
} catch (MalformedURLException e) {
249256
MessageDialog.openError(window.getShell(), WorkbenchMessages.OpenWorkspaceAction_invalidWorkspacePath,

‎bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/WorkspaceLock.java‎

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,14 @@
1313
*******************************************************************************/
1414
package org.eclipse.ui.internal;
1515

16+
import java.io.File;
17+
import java.io.IOException;
1618
import java.io.InputStream;
19+
import java.io.RandomAccessFile;
1720
import java.net.URISyntaxException;
1821
import java.net.URL;
22+
import java.nio.channels.FileLock;
23+
import java.nio.channels.OverlappingFileLockException;
1924
import java.nio.file.Files;
2025
import java.nio.file.Path;
2126
import java.util.Properties;
@@ -101,7 +106,8 @@ public static String getWorkspaceLockDetails(URL workspaceUrl) {
101106
}
102107

103108
/**
104-
* Returns the lock file.
109+
* Returns the lock <b>info</b> file (the file where information about last
110+
* workspace lock owner is saved).
105111
*
106112
* @param workspaceUrl the <code>URL</code> of selected workspace
107113
* @return the path to the <code>.lock_info</code> file within the specified
@@ -117,6 +123,25 @@ public static Path getLockInfoFile(URL workspaceUrl) {
117123
}
118124
}
119125

126+
/**
127+
* Returns the workspace lock file for given workspace URL (the file which is
128+
* actually <b>locked</b> if an Eclipse application is using this workspace).
129+
*
130+
* @param workspaceUrl the <code>URL</code> of selected workspace
131+
* @return the path to the <code>.lock</code> file within the specified
132+
* workspace, or <code> null</code> if the workspace URL cannot be
133+
* converted to a valid URI
134+
*/
135+
public static Path getLockFile(URL workspaceUrl) {
136+
// See org.eclipse.osgi.internal.location.BasicLocation.DEFAULT_LOCK_FILENAME
137+
Path lockFile = Path.of(".metadata", ".lock"); //$NON-NLS-1$ //$NON-NLS-2$
138+
try {
139+
return Path.of(URIUtil.toURI(workspaceUrl)).resolve(lockFile);
140+
} catch (URISyntaxException e) {
141+
return null;
142+
}
143+
}
144+
120145
/**
121146
* Opens an error dialog indicating that the selected workspace is locked by
122147
* another Eclipse instance.
@@ -133,11 +158,58 @@ public static Path getLockInfoFile(URL workspaceUrl) {
133158
*/
134159
public static void showWorkspaceLockedDialog(Shell shell, String workspacePath, String workspaceLockOwner) {
135160
String lockMessage = NLS.bind(WorkbenchMessages.IDEApplication_workspaceCannotLockMessage2, workspacePath);
136-
String wsLockedError = lockMessage + System.lineSeparator() + System.lineSeparator()
137-
+ NLS.bind(WorkbenchMessages.IDEApplication_workspaceLockMessage, workspaceLockOwner);
138-
161+
String wsLockedError = lockMessage;
162+
if (workspaceLockOwner != null && !workspaceLockOwner.isBlank()) {
163+
String lockDetails = NLS.bind(WorkbenchMessages.IDEApplication_workspaceLockMessage, workspaceLockOwner);
164+
wsLockedError += System.lineSeparator() + System.lineSeparator() + lockDetails;
165+
}
139166
MessageDialog.openError(shell,
140167
WorkbenchMessages.IDEApplication_workspaceCannotLockTitle, wsLockedError);
141168
}
142169

170+
/**
171+
* Checks if the given workspace path is locked by another Eclipse instance.
172+
*
173+
* @param workspaceUrl the <code>URL</code> of workspace to check for lock
174+
* @return <code>true</code> if the workspace is locked, <code>false</code>
175+
* otherwise
176+
*/
177+
public static boolean isWorkspaceLocked(URL workspaceUrl) {
178+
Path lockFile = getLockFile(workspaceUrl);
179+
if (lockFile == null || !Files.exists(lockFile)) {
180+
return false;
181+
}
182+
return isLocked(lockFile.toFile());
183+
}
184+
185+
/**
186+
* Follows the same strategy as
187+
* <code>org.eclipse.osgi.internal.location.Locker_JavaNio#isLocked()</code>,
188+
* trying to lock a file using Java NIO to check if the file is locked or not
189+
* already.
190+
*
191+
* @return <code>true</code> if the file is definitely locked by any process,
192+
* <code>false</code> otherwise
193+
*/
194+
private static boolean isLocked(File lockFile) {
195+
try (RandomAccessFile temp = new RandomAccessFile(lockFile, "rw")) { //$NON-NLS-1$
196+
try {
197+
try (FileLock tempLock = temp.getChannel().tryLock(0, 1, false)) {
198+
if (tempLock != null) {
199+
// able to lock: it was not locked before
200+
return false;
201+
}
202+
// is locked by some process
203+
return true;
204+
}
205+
} catch (OverlappingFileLockException e) {
206+
// is locked by some process
207+
return true;
208+
}
209+
} catch (IOException e) {
210+
// assume not locked if we have any troubles getting access to it
211+
return false;
212+
}
213+
}
214+
143215
}

‎tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/UiTestSuite.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@
108108
WorkbenchDatabindingTest.class,
109109
ChooseWorkspaceDialogTests.class,
110110
ViewerItemsLimitTest.class,
111-
OpenCloseTest.class
111+
OpenCloseTest.class,
112+
WorkspaceLockTest.class
112113
})
113114
public class UiTestSuite {
114115
}

‎tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/WorkspaceLockTest.java‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2026 Andrey Loskutov <loskutov@gmx.de>.
3+
*
4+
* This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Public License 2.0
6+
* which accompanies this distribution, and is available at
7+
* https://www.eclipse.org/legal/epl-2.0/
8+
*
9+
* SPDX-License-Identifier: EPL-2.0
10+
*
11+
* Contributors:
12+
* Andrey Loskutov <loskutov@gmx.de> - initial API and implementation
13+
*******************************************************************************/
14+
package org.eclipse.ui.tests;
15+
16+
import static org.junit.jupiter.api.Assertions.assertEquals;
17+
import static org.junit.jupiter.api.Assertions.assertFalse;
18+
import static org.junit.jupiter.api.Assertions.assertNotNull;
19+
import static org.junit.jupiter.api.Assertions.assertNull;
20+
import static org.junit.jupiter.api.Assertions.assertTrue;
21+
22+
import java.io.File;
23+
import java.io.IOException;
24+
import java.io.RandomAccessFile;
25+
import java.net.URL;
26+
import java.nio.channels.FileLock;
27+
import java.nio.channels.OverlappingFileLockException;
28+
import java.nio.file.Files;
29+
import java.nio.file.Path;
30+
import java.util.Properties;
31+
32+
import org.eclipse.core.tests.harness.FileSystemHelper;
33+
import org.eclipse.ui.internal.WorkspaceLock;
34+
import org.junit.jupiter.api.Test;
35+
36+
/**
37+
* Tests for {@link org.eclipse.ui.internal.WorkspaceLock}
38+
*/
39+
public class WorkspaceLockTest {
40+
41+
Path tempDir = FileSystemHelper.getRandomLocation().toPath().resolve("WorkspaceLockTest");
42+
43+
/**
44+
* Test method for {@link org.eclipse.ui.internal.WorkspaceLock#getWorkspaceLockDetails(java.net.URL)}.
45+
*/
46+
@Test
47+
public void testGetWorkspaceLockDetails() throws Exception {
48+
URL workspaceUrl = tempDir.toUri().toURL();
49+
50+
// Test when no lock info file exists
51+
String details = WorkspaceLock.getWorkspaceLockDetails(workspaceUrl);
52+
assertNull(details, "Should return null when no lock info file exists");
53+
54+
// Create .metadata/.lock_info with properties
55+
Path metadataDir = tempDir.resolve(".metadata");
56+
Files.createDirectories(metadataDir);
57+
Path lockInfoFile = metadataDir.resolve(".lock_info");
58+
59+
Properties props = new Properties();
60+
props.setProperty(WorkspaceLock.USER, "testuser");
61+
props.setProperty(WorkspaceLock.HOST, "testhost");
62+
props.setProperty(WorkspaceLock.DISPLAY, ":0");
63+
props.setProperty(WorkspaceLock.PROCESS_ID, "1234");
64+
try (var os = Files.newOutputStream(lockInfoFile)) {
65+
props.store(os, null);
66+
}
67+
68+
details = WorkspaceLock.getWorkspaceLockDetails(workspaceUrl);
69+
assertNotNull(details, "Should return details when lock info file exists");
70+
assertTrue(details.contains("testuser"), "Should contain user info");
71+
assertTrue(details.contains("testhost"), "Should contain host info");
72+
assertTrue(details.contains(":0"), "Should contain display info");
73+
assertTrue(details.contains("1234"), "Should contain PID info");
74+
}
75+
76+
/**
77+
* Test method for {@link org.eclipse.ui.internal.WorkspaceLock#getLockInfoFile(java.net.URL)}.
78+
*/
79+
@Test
80+
public void testGetLockInfoFile() throws Exception {
81+
URL workspaceUrl = tempDir.toUri().toURL();
82+
Path lockInfoFile = WorkspaceLock.getLockInfoFile(workspaceUrl);
83+
assertNotNull(lockInfoFile, "Should return a path");
84+
assertEquals(tempDir.resolve(".metadata").resolve(".lock_info"), lockInfoFile,
85+
"Should point to .metadata/.lock_info");
86+
}
87+
88+
/**
89+
* Test method for {@link org.eclipse.ui.internal.WorkspaceLock#getLockFile(java.net.URL)}.
90+
*/
91+
@Test
92+
public void testGetLockFile() throws Exception {
93+
URL workspaceUrl = tempDir.toUri().toURL();
94+
Path lockFile = WorkspaceLock.getLockFile(workspaceUrl);
95+
assertNotNull(lockFile, "Should return a path");
96+
assertEquals(tempDir.resolve(".metadata").resolve(".lock"), lockFile, "Should point to .metadata/.lock");
97+
}
98+
99+
/**
100+
* Test method for {@link org.eclipse.ui.internal.WorkspaceLock#isWorkspaceLocked(java.net.URL)}.
101+
*/
102+
@Test
103+
public void testIsWorkspaceLocked() throws Exception {
104+
URL workspaceUrl = tempDir.toUri().toURL();
105+
106+
// Test when no lock file exists
107+
assertFalse(WorkspaceLock.isWorkspaceLocked(workspaceUrl), "Should not be locked when no lock file exists");
108+
109+
// Create .metadata/.lock file & lock it
110+
Path metadataDir = tempDir.resolve(".metadata");
111+
Files.createDirectories(metadataDir);
112+
Path lockFile = metadataDir.resolve(".lock");
113+
try (RandomAccessFile lock = lock(lockFile.toFile())) {
114+
assertTrue(WorkspaceLock.isWorkspaceLocked(workspaceUrl), "Should be locked");
115+
}
116+
}
117+
118+
/**
119+
* Mimics
120+
* {@linkplain org.eclipse.osgi.internal.location.Locker_JavaNio#lock(File)}
121+
* locking a file using Java NIO and returning the RandomAccessFile if
122+
* successful, null if the file is already locked by another process.
123+
*/
124+
static RandomAccessFile lock(File lockFile) {
125+
try {
126+
RandomAccessFile raFile = new RandomAccessFile(lockFile, "rw");
127+
try {
128+
FileLock lock = raFile.getChannel().tryLock(0, 1, false);
129+
if (lock == null) {
130+
raFile.close();
131+
return null;
132+
}
133+
return raFile;
134+
} catch (OverlappingFileLockException e) {
135+
raFile.close();
136+
return null;
137+
}
138+
} catch (IOException e) {
139+
// already locked by some process, should not happen in test
140+
return null;
141+
}
142+
}
143+
}

0 commit comments

Comments
 (0)