Skip to content

Additional symlink loop check for disable_advanced_recursive_link_checks#2809

Merged
iloveeclipse merged 1 commit into
eclipse-platform:masterfrom
iloveeclipse:rec_symlink_fix
Jul 7, 2026
Merged

Additional symlink loop check for disable_advanced_recursive_link_checks#2809
iloveeclipse merged 1 commit into
eclipse-platform:masterfrom
iloveeclipse:rec_symlink_fix

Conversation

@iloveeclipse

Copy link
Copy Markdown
Member

If the (non default) system property -Dorg.eclipse.core.resources.disable_advanced_recursive_link_checks" is set, expensive advanced symlink checks are disabled to avoid performance / OOM issues. This is a known tradeoff between correctness and performance.

This commit adds one more "symlink loop" use case detected by the "non advanced" recursive symlink checks code path.

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.

Pull request overview

This PR improves the “simple” (non-advanced) recursive symlink detection path used when -Dorg.eclipse.core.resources.disable_advanced_recursive_link_checks is enabled, adding coverage for an additional loop scenario while keeping the performance-oriented tradeoff.

Changes:

  • Extend UnifiedTree’s cheap “backwards link” recursion detection to also consider the non-real parent path (to catch loops involving symlinked path segments).
  • Add a new regression test scenario with mutually recursive links across sibling directories and validate project traversal terminates.
  • Adjust an existing symlink regression test to avoid running under the simple implementation (which can hang), and improve assertion messages.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_185247_recursiveLinks.java Adds a new mutually-recursive symlink structure test and refactors helpers to return the imported project for traversal.
resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/localstore/SymlinkResourceTest.java Skips a known-hanging test case when advanced recursive link checks are disabled; improves assertion diagnostics.
resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/localstore/UnifiedTree.java Enhances the non-advanced recursion check to detect additional symlink loop cases by checking both real and non-real parent paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Test Results

    54 files  ±0      54 suites  ±0   58m 55s ⏱️ -10s
 4 687 tests +2   4 665 ✅ +2   22 💤 ±0  0 ❌ ±0 
11 955 runs  +6  11 802 ✅ +6  153 💤 ±0  0 ❌ ±0 

Results for commit 7ffc8a8. ± Comparison against base commit fe3597d.

♻️ This comment has been updated with latest results.

If the (non default) system property
`-Dorg.eclipse.core.resources.disable_advanced_recursive_link_checks"`
is set, expensive advanced symlink checks are disabled to avoid
performance / OOM issues. This is a known tradeoff between correctness
and performance.

This commit adds one more "symlink loop" use case detected by the "non
advanced" recursive symlink checks code path.
@iloveeclipse iloveeclipse merged commit a7c593a into eclipse-platform:master Jul 7, 2026
18 checks passed
@iloveeclipse iloveeclipse deleted the rec_symlink_fix branch July 7, 2026 19:11
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.

2 participants