Review: eclipse.jdt.core#14 stale buffer fix — critical gap in Openable.getBuffer()

[Copilot]

Analysis of carstenartur/eclipse.jdt.core#14 which proposes fixing intermittent source=null failures in AnnotateAssistTest1d8 (eclipse-jdt/eclipse.jdt.ui#736) by detecting stale buffers in ClassFile.openBuffer() and ModularClassFile.openBuffer().

Finding: Fix is in the wrong method

The PR adds stale buffer detection in openBuffer(), but openBuffer() is only called when the cache returns null. For top-level class files (the actual failing case), Openable.getBuffer() finds the stale buffer directly in cache and returns it — openBuffer() is never reached:

// Openable.getBuffer()
IBuffer buffer = getBufferManager().getBuffer(this);  // finds stale buffer
if (buffer == null) {
    buffer = openBuffer(null, info);  // never called — buffer is non-null
}
return buffer;  // returns stale buffer with null contents

Recommended fix

The primary fix must be in Openable.getBuffer():

IBuffer buffer = getBufferManager().getBuffer(this);
if (buffer != null && !(buffer instanceof NullBuffer) && buffer.isClosed()) {
    getBufferManager().removeBuffer(buffer);
    buffer = null;
}

Additional findings

• Test doesn't reproduce the bug — passes without the fix (confirmed by @stephan-herrmann). removeLibrary()/addLibrary() run synchronously, no race window.
• ModularClassFile change is over-scoped — adds a cache lookup that Openable.getBuffer() already performs. Should be reverted.
• Use isClosed() not getCharacters() == null — more semantically precise, avoids false positives on unfilled buffers, consistent with existing bufferChanged() check.

Full analysis in jdt_core_stale_buffer_fix_review.md.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.