fix(symbol): сделать WorkspaceSymbolIndex синглтоном (#4123)#4125
Merged
Conversation
Contributor
📝 WalkthroughWalkthrough
Changesworkspace/symbol ScopeNotActiveException fix
Sequence DiagramsequenceDiagram
participant Client
participant BSLWorkspaceService
participant SymbolProvider
participant ServerContextProvider
participant WorkspaceContextHolder
participant WorkspaceSymbolIndex
Client->>BSLWorkspaceService: symbol(WorkspaceSymbolParams)
BSLWorkspaceService->>SymbolProvider: getSymbols(params, cancelChecker)
SymbolProvider->>ServerContextProvider: getAllContexts()
ServerContextProvider-->>SymbolProvider: Map of ServerContext
loop each ServerContext with non-null workspaceUri
SymbolProvider->>WorkspaceContextHolder: forUri(workspaceUri)
SymbolProvider->>SymbolProvider: collectWorkspaceSymbols(params, cancelChecker)
SymbolProvider->>WorkspaceSymbolIndex: search(params)
WorkspaceSymbolIndex-->>SymbolProvider: symbols for workspace
end
SymbolProvider-->>BSLWorkspaceService: aggregated symbols
BSLWorkspaceService-->>Client: Either.forRight(symbols)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SymbolProviderTest.java`:
- Around line 79-82: The WorkspaceContextHolder is set in the test setup but
never restored, causing context leakage across tests. Add a cleanup method
(using `@After` annotation for JUnit 4 or `@AfterEach` for JUnit 5) that restores
the WorkspaceContextHolder to its previous state by calling the appropriate
reset or clear method on WorkspaceContextHolder. This should be paired with the
setup code where WorkspaceContextHolder.set() is called to ensure the holder is
properly cleaned up after each test execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06d53217-25e4-42bd-aaf4-9d47619c904b
📒 Files selected for processing (4)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SymbolProvider.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SymbolProviderScriptVariantTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SymbolProviderStreamingTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SymbolProviderTest.java
Comment on lines
79
to
82
| // Индекс символов — workspace-scoped: наполняем его под контекстом этого workspace, | ||
| // чтобы getSymbols, обходящий getAllContexts(), нашёл символы в его индексе. | ||
| WorkspaceContextHolder.set(workspaceContext.getWorkspaceUri()); | ||
| workspaceContext.populateContext(); |
Contributor
There was a problem hiding this comment.
Restore workspace context after test setup to avoid leakage
Line 81 sets WorkspaceContextHolder but never restores it. This can leak context across tests and make failures order-dependent.
Suggested fix
- WorkspaceContextHolder.set(workspaceContext.getWorkspaceUri());
- workspaceContext.populateContext();
+ try (var ignored = WorkspaceContextHolder.forUri(workspaceContext.getWorkspaceUri())) {
+ workspaceContext.populateContext();
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SymbolProviderTest.java`
around lines 79 - 82, The WorkspaceContextHolder is set in the test setup but
never restored, causing context leakage across tests. Add a cleanup method
(using `@After` annotation for JUnit 4 or `@AfterEach` for JUnit 5) that restores
the WorkspaceContextHolder to its previous state by calling the appropriate
reset or clear method on WorkspaceContextHolder. This should be paired with the
setup code where WorkspaceContextHolder.set() is called to ensure the holder is
properly cleaned up after each test execution.
workspace/symbol падал с ScopeNotActiveException: индекс был @WorkspaceScope, а запрос выполняется на потоке workspaceServiceExecutor без установленного JSON-RPC диспетчером workspace-контекста, поэтому scoped proxy не резолвился. Регресс rc.1 → rc.2: в rc.1 индекса не было (workspace/symbol шёл обходом документов и работал кросс-воркспейсно), индекс появился в rc.2 уже scoped. workspace/symbol по протоколу LSP — кросс-воркспейсный запрос, а индекс самодостаточен (не обращается ни к одному workspace-scoped бину) и держит жизненный цикл на per-document событиях, поэтому workspace-scope был лишним и только ломал доступ с потока без контекста. Снят: индекс — обычный singleton с единым trie по всем рабочим областям; закрытие/удаление области штатно вычищает её документы через ServerContextDocumentRemovedEvent. SymbolProvider и BSLWorkspaceService остаются без изменений относительно develop. Добавлен регрессионный тест: workspace/symbol обслуживается с потока без workspace-контекста. Closes #4123 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
nixel2007
force-pushed
the
fix/workspace-symbol-scope-context
branch
from
June 15, 2026 16:36
f781679 to
aaa81c7
Compare
nixel2007
changed the title
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Проблема
workspace/symbolстабильно падает с-32603 Internal error(issue #4123):Запрос выполняется на потоке
workspaceServiceExecutor, которому JSON-RPC диспетчер не устанавливает workspace-контекст, аWorkspaceSymbolIndexбыл@WorkspaceScope→ scoped proxy не резолвится.Регресс rc.1 → rc.2: в rc.1 индекса ещё не было —
workspace/symbolшёл обходом документов и работал кросс-воркспейсно. Индекс появился в rc.2 уже@WorkspaceScope, что и сломало запрос.Решение
Снят
@WorkspaceScope— индекс стал обычным синглтоном.Обоснование, почему scope здесь лишний:
workspace/symbolпо протоколу LSP — кросс-воркспейсный запрос: единый индекс по всем рабочим областям это и есть желаемое поведение (как в rc.1), изолировать области в выдаче не нужно;index()/search()не обращаются ни к одному@WorkspaceScope-бину, workspace-контекст потока ему не нужен;Changed/Cleared/Closed/Removed);removeWorkspaceштатно рассылаетServerContextDocumentRemovedEventпо каждому документу → закрытие/удаление области вычищает её документы без per-workspace teardown'а;ReadWriteLock+ConcurrentMap), конкурентная индексация из populate-пулов разных областей корректна.SymbolProviderиBSLWorkspaceServiceостаются без изменений относительно develop — провайдер просто дёргает один индекс.Тесты
BSLWorkspaceServiceTest#symbol_worksFromThreadWithoutWorkspaceContext: воспроизводит прод-условие (вызывающий поток без workspace-контекста). До фикса падал сScopeNotActiveException, после — зелёный.WorkspaceSymbolIndexTest,SymbolProvider*Test,BSLWorkspaceServiceTestпроходят (тестовыйliteCleanupчистит синглтон-индекс через те же document-события).Closes #4123
🤖 Generated with Claude Code