Skip to content

fix(core): resolve read-lock deadlock and harden config loading - #1491

Merged
tangcent merged 1 commit into
tangcent:masterfrom
alona789:master
Nov 9, 2026
Merged

tangcent merged 1 commit into
tangcent:masterfrom
alona789:master

Conversation

@alona789

@alona789 alona789 commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Description

Fix an IDE freeze caused by a read-lock deadlock in field ordering, and make config loading more robust.

When field.order.with is configured, DefaultPsiClassHelper.applyFieldOrdering called runBlocking inside the sortedWith comparator. The calling thread already held a read action, and the rule script it waited for (Groovy it.name() → readSync) had to acquire the read permit again. Once a write action was pending, new read permits were blocked, so the read-lock holder, the script coroutine and the pending writer waited on each other. The EDT then hung in runReadAction / runWriteAction for 30+ seconds.

Config reloads on the plugin's background thread also read FileEditorManager and ProjectFileIndex without a read action, and a single failing config source discarded the entries of every other source.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

  • Refactoring (no functional changes)

  • Documentation update

  • Performance improvement

  • Test coverage improvement

Related Issues

Fixes #

Affected Area

  • YApi export

  • Postman export

  • Markdown export

  • Send HTTP request (Call)

  • API scanning / dashboard

  • Rule engine / custom rules / config

  • Settings / UI

  • Other / not user-facing

Changes Made

  • DefaultPsiClassHelper: remove runBlocking from the field.order.with comparator and sort with the new suspendMergeSort, a stable merge sort that accepts a suspending comparator. Rule evaluation now suspends instead of blocking the thread that holds the read lock. An inconsistent user comparator no longer throws Comparison method violates its general contract.
  • ConfigTextParser.activeModuleContentRoot: wrap FileEditorManager.selectedFiles and ProjectFileIndex access in readSync.
  • LayeredConfigReader: catch failures per config source, log the failed source, and keep loading the remaining sources.
  • Add SuspendMergeSortTest (ordering, empty/single element, stability, suspending comparator on another dispatcher, inconsistent comparator).

How I Tested

  • ./gradlew test passes

  • Manual verification in a sandbox IDE (./gradlew runIde) — describe the entry action exercised

    Ran the tests and used the plugin with `field.order.with` configured; the IDE no longer froze (previously it hung for 30+ seconds while scanning API endpoints and moving the mouse in the editor).
    
  • New tests added for new functionality

Root cause was identified from IDE freeze thread dumps: EasyAPI-background blocked in DefaultPsiClassHelper.applyFieldOrdering$lambda$22 → runBlocking while the EDT waited on a read permit with writeActionPending:true.

Architecture & Threading Checklist

  • New code lives in the right bucket (core/psi, core/config, existing files); no new core.* → concrete sibling imports

  • PSI/VFS reads wrapped in read {} / readSync {} — ConfigTextParser now uses readSync for editor and file index access; the sort itself performs no PSI reads

  • Boundary classes self-protect every public member — N/A, no boundary class touched

  • PSI writes and UI updates go through write {} / swing {} — N/A, no writes or UI

  • No new launch(Dispatchers.Default) from EDT / startup contexts — none added; the sort stays suspending and no longer blocks a thread

Logging Checklist

  • Exactly one channel per event — a failed config source is reported once through LOG.warn

  • No LOG.error, LOG.debug, LOG.trace, println, or printStackTrace() in production code

  • Throwables passed as the last argument; no silent runCatching{}.getOrNull() or empty catch — the runCatching in LayeredConfigReader logs the failure with the throwable

General Checklist

  • My code follows the project's architecture principles

  • I have performed a self-review of my code

  • I have commented my code, particularly in hard-to-understand areas

  • I have made corresponding changes to the documentation

  • My changes generate no new warnings

  • I have added tests that prove my fix is effective or that my feature works

  • New and existing unit tests pass locally with my changes

Screenshots (if applicable)

N/A

Additional Notes

  • No other runBlocking usage exists under src/main, so this was the only occurrence.
  • The deadlock itself is not covered by an automated test, because reproducing it needs a pending write action while a read action is held, which is hard to do reliably in a light fixture. BuiltInExtensionFieldOrderScenarioTest still covers field.order.with end to end.

- Wrap FileEditorManager and ProjectFileIndex access in readSync so config reloads on background threads hold a proper read action
- Replace runBlocking inside the field.order.with comparator with a stable suspending merge sort (suspendMergeSort), so rule scripts no longer block a thread that holds the read lock
- Isolate config source failures in LayeredConfigReader so one failing source no longer discards entries from the others, and log the failed source
- Add unit tests for suspendMergeSort
@github-actions github-actions Bot added the type: bug Something isn't working label Nov 4, 2026
@codecov

codecov Bot commented Nov 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.52941% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.975%. Comparing base (5b4f076) to head (1b140e0).

Files with missing lines Patch % Lines
...ent/easyapi/core/config/parser/ConfigTextParser.kt 41.667% 1 Missing and 6 partials ⚠️
...angcent/easyapi/core/config/LayeredConfigReader.kt 66.667% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##            master     #1491       +/-   ##
=============================================
+ Coverage   69.934%   69.975%   +0.041%     
=============================================
  Files          518       518               
  Lines        30383     30398       +15     
  Branches      7147      7153        +6     
=============================================
+ Hits         21248     21271       +23     
+ Misses        6374      6370        -4     
+ Partials      2761      2757        -4     
Flag Coverage Δ
unittests 69.975% <73.529%> (+0.041%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...tangcent/easyapi/core/psi/DefaultPsiClassHelper.kt 66.425% <100.000%> (+0.869%) ⬆️
...angcent/easyapi/core/config/LayeredConfigReader.kt 94.000% <66.667%> (-3.872%) ⬇️
...ent/easyapi/core/config/parser/ConfigTextParser.kt 85.470% <41.667%> (+0.125%) ⬆️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b4f076...1b140e0. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tangcent

tangcent commented Nov 4, 2026

Copy link
Copy Markdown
Owner

@alona789 thanks for tracking down the IDE freeze! the suspending sort looks like the right fix, and all five sort tests passed locally.

Before merge: please rethrow CancellationException in LayeredConfigReader.reload() and check currentCoroutineContext().ensureActive() in DefaultConfigReader.reload() just before replacing the active config. if a newer change cancels a reload while it’s waiting on an IntelliJ read action to check whether an extension applies, the new runCatching can swallow the cancellation. the cancelled reload can then publish a config missing that extension’s rules and notify listeners. the final check also covers the remote config loader, which can already catch cancellation and return a fallback.

Non-blocking suggestions:

  • the full CI suite appears green, but three ${module_path} cases in ConfigTextParserTest failed in a focused local run because the new readSync call requires an IntelliJ application. consider moving those cases to an IDE fixture so they also pass in isolation.
  • consider moving suspendMergeSort into a sibling SuspendMergeSort.kt and keeping it internal. it has its own tests and doesn’t depend on PSI, so this would make DefaultPsiClassHelper.kt easier to navigate.

@tangcent tangcent added the status: feedback required feedback required label Nov 4, 2026
@tangcent
tangcent merged commit 4e071e0 into tangcent:master Nov 9, 2026
14 checks passed
@tangcent tangcent mentioned this pull request Nov 10, 2026
22 of 33 tasks
@github-actions

Copy link
Copy Markdown
Contributor

A PR (#1495) has been opened to fix this issue. You can download the packaged plugin from the GitHub Actions workflow run by clicking on the "Artifacts" dropdown:

  • easy-yapi-3.2.6.252.0.zip — IntelliJ IDEA 2025.2+ (officially supported)
  • easy-yapi-3.2.6.221.251.zip — IntelliJ IDEA 2022.1 – 2025.1 (best-effort, not verified on older IDEs)

Each artifact is a ready-to-install plugin zip — pick the one matching your IDEA version and install it via Settings → Plugins → ⚙ → Install Plugin from Disk. No extraction needed.

tangcent added a commit that referenced this pull request Nov 10, 2026
* fix(core): propagate config reload cancellation

Editing config files in quick succession could leave the plugin
running with a partial config: a superseded reload still published
the sources it had managed to read and notified listeners, so rules
from the remaining sources briefly disappeared.

Root cause: `LayeredConfigReader.reload()` wraps each source in
`runCatching` to isolate failures. A `CancellationException` raised
while waiting on an IntelliJ read action was logged as an ordinary
source failure, so the cancelled reload ran to completion.
`DefaultConfigReader.reload()` then replaced the active delegate and
fired `ConfigReloadListener` without checking whether its coroutine
was still active. The remote config loader can hide the cancellation
entirely: it catches it and returns a fallback.

Solution: rethrow `CancellationException` from the per-source
`onFailure` while keeping warn-and-skip for real source failures,
and call `currentCoroutineContext().ensureActive()` after the new
delegate has loaded but before it becomes active and listeners are
notified. Add tests for both paths plus a guard that ordinary
failures are still isolated. Both guards were reverse-verified:
removing either one makes the new tests fail.

* refactor(psi): extract suspendMergeSort

Move the suspending merge sort out of `DefaultPsiClassHelper.kt` into
a sibling `SuspendMergeSort.kt`, keeping it `internal`. It has no PSI
dependency and already has its own tests. Translate its KDoc and the
comparator test comment to English, matching the rest of the
codebase.

* test(config): move module_path tests to a fixture

The three `${module_path}` cases call `readSync`, which needs an
IntelliJ application. They only passed when another fixture test had
already set one up, so a focused run of `ConfigTextParserTest` failed.
Move them to `ConfigTextParserModulePathTest`, based on the project's
light IDE fixture, so they pass in isolation too.

* docs(psi): translate field ordering comments

Translate the last three Chinese comments added by #1491 at the
`suspendMergeSort` call site. All six Chinese comment lines from that
PR are now English.

Impact: ordinary source-failure isolation is unchanged. Apart from
the two cancellation guards, this branch is test, refactor and
comment only. The changes above address the review on #1491: both
"Before merge" requests and both non-blocking suggestions. Verified
with `./gradlew cleanTest test`: 7669 tests, 0 failures, 0 errors,
10 skipped.

Refs: #1491
@tangcent tangcent mentioned this pull request Nov 10, 2026
tangcent added a commit to tangcent/easy-api that referenced this pull request Nov 11, 2026
* fix(core): propagate config reload cancellation

Editing config files in quick succession could leave the plugin
running with a partial config: a superseded reload still published
the sources it had managed to read and notified listeners, so rules
from the remaining sources briefly disappeared.

Root cause: `LayeredConfigReader.reload()` wraps each source in
`runCatching` to isolate failures. A `CancellationException` raised
while waiting on an IntelliJ read action was logged as an ordinary
source failure, so the cancelled reload ran to completion.
`DefaultConfigReader.reload()` then replaced the active delegate and
fired `ConfigReloadListener` without checking whether its coroutine
was still active. The remote config loader can hide the cancellation
entirely: it catches it and returns a fallback.

Solution: rethrow `CancellationException` from the per-source
`onFailure` while keeping warn-and-skip for real source failures,
and call `currentCoroutineContext().ensureActive()` after the new
delegate has loaded but before it becomes active and listeners are
notified. Add tests for both paths plus a guard that ordinary
failures are still isolated. Both guards were reverse-verified:
removing either one makes the new tests fail.

* refactor(psi): extract suspendMergeSort

Move the suspending merge sort out of `DefaultPsiClassHelper.kt` into
a sibling `SuspendMergeSort.kt`, keeping it `internal`. It has no PSI
dependency and already has its own tests. Translate its KDoc and the
comparator test comment to English, matching the rest of the
codebase.

* test(config): move module_path tests to a fixture

The three `${module_path}` cases call `readSync`, which needs an
IntelliJ application. They only passed when another fixture test had
already set one up, so a focused run of `ConfigTextParserTest` failed.
Move them to `ConfigTextParserModulePathTest`, based on the project's
light IDE fixture, so they pass in isolation too.

* docs(psi): translate field ordering comments

Translate the last three Chinese comments added by tangcent/easy-yapi#1491 at the
`suspendMergeSort` call site. All six Chinese comment lines from that
PR are now English.

Impact: ordinary source-failure isolation is unchanged. Apart from
the two cancellation guards, this branch is test, refactor and
comment only. The changes above address the review on tangcent/easy-yapi#1491: both
"Before merge" requests and both non-blocking suggestions. Verified
with `./gradlew cleanTest test`: 7669 tests, 0 failures, 0 errors,
10 skipped.

Refs: tangcent/easy-yapi#1491
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: feedback required feedback required type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants