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
Contributor

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   56m 25s ⏱️ - 3m 18s
 4 863 tests +1   4 841 ✅ +1   22 💤 ±0  0 ❌ ±0 
12 480 runs  +3  12 326 ✅ +3  154 💤 ±0  0 ❌ ±0 

Results for commit e68f885. ± Comparison against base commit 9dd1e78.

♻️ This comment has been updated with latest results.

@ravnskjaer
ravnskjaer force-pushed the fix-123-documentmerger-npe branch from 693ea16 to 54edb62 Compare October 9, 2026 18:06
@ravnskjaer

Copy link
Copy Markdown
Contributor Author

@iloveeclipse I’ve added the regression test Copilot asked for (CompareEditorOpenRaceTest). The GitHub workflows are awaiting approval, and the Jenkins build timed out for this PR too. Could you approve the workflows and retrigger Jenkins?

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 54edb62 to e68f885 Compare October 11, 2026 19:03
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