Skip to content

Commit 972aa28

Browse files
Copilotcarstenartur
andcommitted
Fix stale buffer reuse race condition in ClassFile and ModularClassFile
Add validation to detect and remove stale cached buffers when jar files are recreated with the same path. This prevents reusing buffers with null contents that can cause source=null errors in tests. Co-authored-by: carstenartur <3164220+carstenartur@users.noreply.github.com>
1 parent 6ca070c commit 972aa28

3 files changed

Lines changed: 93 additions & 4 deletions

File tree

org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ClassFileTests.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1686,4 +1686,61 @@ public void run(){
16861686
}
16871687
}
16881688

1689+
/*
1690+
* Ensures that stale buffers from deleted/recreated jars are properly detected and replaced.
1691+
* This test simulates the race condition where a jar file is deleted and recreated with the same path,
1692+
* and verifies that stale cached buffers are not reused.
1693+
* See https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/736
1694+
*/
1695+
public void testStaleBufferAfterJarRecreation() throws CoreException, IOException {
1696+
IJavaProject project = null;
1697+
try {
1698+
// Create a test project
1699+
project = createJavaProject("TestStaleBuffer", new String[0], new String[] {"JCL18_LIB"}, "", JavaCore.VERSION_1_8);
1700+
1701+
// Create initial jar with source
1702+
String[] pathAndContents = new String[] {
1703+
"pack/age/X.java",
1704+
"package pack.age;\n" +
1705+
"public interface X {\n" +
1706+
" String test();\n" +
1707+
"}"
1708+
};
1709+
addLibrary(project, "testlib.jar", "testlibsrc.zip", pathAndContents, JavaCore.VERSION_1_8);
1710+
1711+
// Get the class file and trigger buffer creation
1712+
IPackageFragmentRoot root = project.getPackageFragmentRoot(project.getProject().getFile("testlib.jar"));
1713+
IOrdinaryClassFile classFile1 = root.getPackageFragment("pack.age").getOrdinaryClassFile("X.class");
1714+
String source1 = classFile1.getSource();
1715+
assertNotNull("Source should be available for first jar", source1);
1716+
assertTrue("Source should contain 'test()' method", source1.contains("test()"));
1717+
1718+
// Delete and recreate the jar with different content
1719+
removeLibrary(project, "testlib.jar", "testlibsrc.zip");
1720+
String[] newPathAndContents = new String[] {
1721+
"pack/age/X.java",
1722+
"package pack.age;\n" +
1723+
"public interface X {\n" +
1724+
" String newMethod();\n" +
1725+
"}"
1726+
};
1727+
addLibrary(project, "testlib.jar", "testlibsrc.zip", newPathAndContents, JavaCore.VERSION_1_8);
1728+
1729+
// Get the class file again (same path, but new jar)
1730+
root = project.getPackageFragmentRoot(project.getProject().getFile("testlib.jar"));
1731+
IOrdinaryClassFile classFile2 = root.getPackageFragment("pack.age").getOrdinaryClassFile("X.class");
1732+
String source2 = classFile2.getSource();
1733+
1734+
// Verify that we get the new content, not stale buffer
1735+
assertNotNull("Source should be available for recreated jar", source2);
1736+
assertFalse("Source should not contain old 'test()' method", source2.contains("test()"));
1737+
assertTrue("Source should contain new 'newMethod()'", source2.contains("newMethod()"));
1738+
1739+
} finally {
1740+
if (project != null) {
1741+
deleteProject(project);
1742+
}
1743+
}
1744+
}
1745+
16891746
}

org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ClassFile.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,21 @@ protected IBuffer openBuffer(IProgressMonitor pm, IElementInfo info) throws Java
454454
// Check the cache for the top-level type first
455455
IType outerMostEnclosingType = getOuterMostEnclosingType();
456456
IBuffer buffer = getBufferManager().getBuffer(outerMostEnclosingType.getClassFile());
457+
458+
// Validate the cached buffer is still valid (not stale from a deleted/recreated jar)
459+
if (buffer != null) {
460+
if (buffer instanceof NullBuffer) {
461+
// NullBuffer is valid - it represents a class file without source
462+
return null;
463+
}
464+
// Check if buffer contents are still valid
465+
if (buffer.getCharacters() == null) {
466+
// Stale buffer - remove from cache and recreate
467+
getBufferManager().removeBuffer(buffer);
468+
buffer = null;
469+
}
470+
}
471+
457472
if (buffer == null) {
458473
SourceMapper mapper = getSourceMapper();
459474
IBinaryType typeInfo = info instanceof IBinaryType ? (IBinaryType) info : null;

org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModularClassFile.java

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,28 @@ public ICompilationUnit getWorkingCopy(WorkingCopyOwner owner, IProgressMonitor
254254
*/
255255
@Override
256256
protected IBuffer openBuffer(IProgressMonitor pm, IElementInfo info) throws JavaModelException {
257-
SourceMapper mapper = getSourceMapper();
258-
if (mapper != null) {
259-
return mapSource(mapper);
257+
// First check if there's an existing buffer in the cache
258+
IBuffer buffer = getBufferManager().getBuffer(this);
259+
260+
// Validate the cached buffer is still valid
261+
if (buffer != null) {
262+
if (buffer instanceof NullBuffer) {
263+
return null;
264+
}
265+
if (buffer.getCharacters() == null) {
266+
// Stale buffer - remove from cache and recreate
267+
getBufferManager().removeBuffer(buffer);
268+
buffer = null;
269+
}
260270
}
261-
return null;
271+
272+
if (buffer == null) {
273+
SourceMapper mapper = getSourceMapper();
274+
if (mapper != null) {
275+
buffer = mapSource(mapper);
276+
}
277+
}
278+
return buffer;
262279
}
263280

264281
/** Loads the buffer via SourceMapper, and maps it in SourceMapper */

0 commit comments

Comments
 (0)