Skip to content

Fix NPE in DocumentMerger.doDiff #123 - #2990

Open
ravnskjaer wants to merge 1 commit into
eclipse-platform:masterfrom
ravnskjaer:fix-123-documentmerger-npe
Open

ravnskjaer wants to merge 1 commit into
eclipse-platform:masterfrom
ravnskjaer:fix-123-documentmerger-npe

Conversation

@ravnskjaer

Copy link
Copy Markdown

The "still initializing" timer and the initialization job could both create the contents of the same compare editor input. One of the two content trees stayed alive when the editor switched to another input, and its viewer later diffed against the disposed input.

Fixes #123

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The timing-sensitive fix lacks a deterministic regression test covering the reported race.

1 open finding
What changed in this PR

Prevents duplicate compare content trees and the resulting DocumentMerger NPE during asynchronous initialization.

Changes:

  • Detects re-entrant control creation.
  • Skips duplicate content creation while a control is already being built.
File Description
CompareEditor.java Guards the initialization callback against duplicate compare controls.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Test Results

    54 files  ±0      54 suites  ±0   57m 35s ⏱️ +56s
 4 849 tests ±0   4 827 ✅ ±0   22 💤 ±0  0 ❌ ±0 
12 438 runs  ±0  12 284 ✅ ±0  154 💤 ±0  0 ❌ ±0 

Results for commit 693ea16. ± Comparison against base commit 8fa7d88.

The "still initializing" timer and the initialization job could both
create the contents of the same compare editor input. One of the two
content trees stayed alive when the editor switched to another input,
and its viewer later diffed against the disposed input.

Add a regression test and a DisplayUtils helper for driving the event
loop in tests.

Fixes eclipse-platform#123
@ravnskjaer
ravnskjaer force-pushed the fix-123-documentmerger-npe branch from 693ea16 to 54edb62 Compare October 9, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NPE in org.eclipse.compare.internal.merge.DocumentMerger.doDiff(DocumentMerger.java:451)

2 participants