Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 3, 2025

Описание

NoSuchElementException возникала в getMethodNameByLocation() при вызове orElseThrow() когда findTerminalNodeContainsPosition() возвращал пустой Optional (при рассинхронизации индекса ссылок и AST).

Изменения:

  • getMethodNameByLocation() теперь возвращает Optional<String> вместо выброса исключения
  • getReferenceToMethodCall() использует flatMap для корректной обработки пустого результата
// Было:
private static String getMethodNameByLocation(ParserRuleContext node, Range range) {
  return Trees.findTerminalNodeContainsPosition(node, range.getStart())
    .map(ParseTree::getText)
    .orElseThrow();  // ← NoSuchElementException
}

// Стало:
private static Optional<String> getMethodNameByLocation(ParserRuleContext node, Range range) {
  return Trees.findTerminalNodeContainsPosition(node, range.getStart())
    .map(ParseTree::getText);
}

Связанные задачи

Closes https://1c-syntax.sentry.io/issues/7081340854/

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (присутствуют файлы для обоих языков, для русского заполнено все подробно, перевод на английский можно опустить)

Дополнительно

Все 963 теста проходят успешно. CodeQL проверка не выявила проблем безопасности.

Original prompt

This section details on the original issue you should resolve

<issue_title>NoSuchElementException: No value present</issue_title>
<issue_description>Sentry Issue: BSL-LANGUAGE-SERVER-KG

NoSuchElementException: No value present
    at com.github._1c_syntax.bsl.languageserver.diagnostics.MissingCommonModuleMethodDiagnostic.getMethodNameByLocation(MissingCommonModuleMethodDiagnostic.java:60)
    at com.github._1c_syntax.bsl.languageserver.diagnostics.MissingCommonModuleMethodDiagnostic.getReferenceToMethodCall(MissingCommonModuleMethodDiagnostic.java:93)
    at com.github._1c_syntax.bsl.languageserver.diagnostics.MissingCommonModuleMethodDiagnostic.check(MissingCommonModuleMethodDiagnostic.java:74)
    at com.github._1c_syntax.bsl.languageserver.diagnostics.AbstractDiagnostic.getDiagnostics_aroundBody0(AbstractDiagnostic.java:44)
    at com.github._1c_syntax.bsl.languageserver.diagnostics.AbstractDiagnostic$AjcClosure1.run(AbstractDiagnostic.java:1)
...
(28 additional frame(s) were not displayed)
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@nixel2007</author><body>

java.util.NoSuchElementException: No value present
at java.util.Optional.orElseThrow(Optional.java:377)
at com.github._1c_syntax.bsl.languageserver.diagnostics.MissingCommonModuleMethodDiagnostic.getMethodNameByLocation(MissingCommonModuleMethodDiagnostic.java:60)
at com.github._1c_syntax.bsl.languageserver.diagnostics.MissingCommonModuleMethodDiagnostic.getReferenceToMethodCall(MissingCommonModuleMethodDiagnostic.java:93)
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:197)
at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:197)
at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:197)
at java.util.concurrent.ConcurrentHashMap$KeySpliterator.forEachRemaining(ConcurrentHashMap.java:3589)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:632)
at com.github._1c_syntax.bsl.languageserver.diagnostics.MissingCommonModuleMethodDiagnostic.check(MissingCommonModuleMethodDiagnostic.java:74)
at com.github._1c_syntax.bsl.languageserver.diagnostics.AbstractDiagnostic.getDiagnostics_aroundBody0(AbstractDiagnostic.java:44)
at com.github._1c_syntax.bsl.languageserver.diagnostics.AbstractDiagnostic$AjcClosure1.run(AbstractDiagnostic.java:1)
at org.aspectj.runtime.reflect.JoinPointImpl.proceed(JoinPointImpl.java:164)
at com.github._1c_syntax.bsl.languageserver.aop.MeasuresAspect.measureBSLDiagnostic(MeasuresAspect.java:63)
at com.github._1c_syntax.bsl.languageserver.diagnostics.AbstractDiagnostic.getDiagnostics(AbstractDiagnostic.java:42)
at com.github._1c_syntax.bsl.languageserver.context.computer.DiagnosticComputer.lambda$internalCompute$1(DiagnosticComputer.java:82)
at java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:288)
at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1716)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
at java.util.stream.Nodes$CollectorTask.doLeaf(Nodes.java:2191)
at java.util.stream.Nodes$CollectorTask.doLeaf(Nodes.java:2157)
at java.util.stream.AbstractTask.compute(AbstractTask.java:327)
at java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:759)
at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:511)
at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1450)
at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:2019)
at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:187)

</comments>


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

Changed getMethodNameByLocation to return Optional<String> instead of
throwing when terminal node at position is not found. Updated caller
to handle empty Optional gracefully.

Co-authored-by: nixel2007 <[email protected]>
Copilot AI changed the title [WIP] Fix NoSuchElementException in diagnostics Fix NoSuchElementException in MissingCommonModuleMethodDiagnostic Dec 3, 2025
Copilot AI requested a review from nixel2007 December 3, 2025 08:24
@nixel2007
Copy link
Member

@copilot в какой момент может произойти рассинхронизация? Проведи детальный анализ многопоточного взаимодействия с учётом блокировок

Copy link
Contributor Author

Copilot AI commented Dec 3, 2025

@copilot в какой момент может произойти рассинхронизация? Проведи детальный анализ многопоточного взаимодейств...

Анализ многопоточного взаимодействия:

Рассинхронизация между LocationRepository и AST возможна в следующих сценариях:

1. Race condition при обновлении документа (наиболее вероятный сценарий)

Thread A (ReferenceIndexFiller.handleEvent):     Thread B (DiagnosticComputer.internalCompute):
──────────────────────────────────────────────  ──────────────────────────────────────────────
index.clearReferences(uri)                      locationRepository.getSymbolOccurrencesByLocationUri(uri)
                                                   ↓ получает SymbolOccurrence из "старого" индекса
↓ начинает строить новый индекс
new MethodSymbolReferenceIndexFinder().visit()    ↓ MissingCommonModuleMethodDiagnostic.check()
                                                     ↓ getMethodNameByLocation(documentContext.getAst(), range)
↓ добавляет новые SymbolOccurrence                      ↓ AST уже обновлён, а Range из старого индекса!
                                                           → findTerminalNodeContainsPosition возвращает empty

2. Архитектурная особенность:

  • LocationRepository.locationsConcurrentHashMap<URI, Set<SymbolOccurrence>>
  • Set<SymbolOccurrence>ConcurrentHashMap.newKeySet()
  • Операция getSymbolOccurrencesByLocationUri(uri) возвращает Stream над мутируемой коллекцией
  • Между итерацией по стриму в check() и чтением AST нет синхронизации

3. Блокировки в DocumentContext:

  • computeLock защищает getAst(), rebuild(), clearSecondaryData()
  • diagnosticsLock защищает computeDiagnostics()
  • Но нет блокировки между ReferenceIndexFiller и DiagnosticComputer

Вывод:
Исправление корректно — graceful handling через Optional является правильным подходом для данного race condition, поскольку:

  1. Добавление синхронизации между индексом и диагностиками сильно усложнит архитектуру
  2. Ситуация редкая и не критичная — просто пропускаем конкретное вхождение символа

@nixel2007
Copy link
Member

@copilot ситуация рассинхронизации на ast частая. Попадает ли reference index filler с эвента под computeLock, стоящий вокруг rebuild? Ну и DiagnosticsComputer закрыт diagnostic compute lock'ом

Copy link
Contributor Author

Copilot AI commented Dec 3, 2025

@copilot ситуация рассинхронизации на ast частая. Попадает ли reference index filler с эвента под computeLock, стоящий вокруг rebuild? ...

Проанализировал детальнее:

ReferenceIndexFiller и computeLock:

  • Event публикуется через @AfterReturning — т.е. ПОСЛЕ завершения rebuild(), когда lock уже освобождён
  • ReferenceIndexFiller.fill() вызывает documentContext.getAst(), который защищён @Locked("computeLock")
  • Таким образом, ReferenceIndexFiller получает консистентный AST

Проблема в другом:

getReferenceToMethodCall():
  ↓ symbolOccurrence.location().getRange()  // Range из LocationRepository (не защищён никаким lock'ом)
  ↓ documentContext.getAst()                // AST защищён computeLock

LocationRepository — отдельный ConcurrentHashMap, не связанный с computeLock/diagnosticsLock. Range берётся из SymbolOccurrence, который был создан в момент предыдущего fill() с предыдущим AST.

Сценарий:

  1. Document A rebuild → event → ReferenceIndexFiller.fill(A) → создаёт SymbolOccurrence с Range_v1
  2. Document A rebuild (новая версия) → AST обновляется на AST_v2
  3. Параллельно DiagnosticComputer запрашивает диагностики для A
  4. check() читает из LocationRepository старый SymbolOccurrence с Range_v1
  5. getAst() возвращает AST_v2
  6. Range_v1 не находится в AST_v2 → empty Optional

diagnosticsLock защищает только computeDiagnostics(), но не синхронизирует обновление LocationRepository с вычислением диагностик.

@nixel2007
Copy link
Member

@copilot тот момент, когда начнёт выполняться второй ребилд, будет висеть computeLock, diagnostics не смогут запросить старый ast

Copy link
Contributor Author

Copilot AI commented Dec 3, 2025

@copilot тот момент, когда начнёт выполняться второй ребилд, будет висеть computeLock, diagnostics не смогут запросить стары...

Согласен — computeLock + diagnosticsLock защищают AST: пока идёт rebuild(), диагностики ждут, и наоборот. Для одного документа рассинхронизации AST быть не должно.

Однако ошибка реально происходит (Sentry). Возможные сценарии:

  1. Битый/нестандартный код: Trees.findTerminalNodeContainsPosition() не находит ноду по позиции, если Range указывает на пустую область (начало > конца, или после EOF)

  2. Проблема в парсере/токенизаторе: если AST невалидный после парсинга (ошибки синтаксиса), позиция может не найтись

  3. Edge case в самой findTerminalNodeContainsPosition(): метод возвращает Optional.empty() при tree.getTokens().isEmpty() или если позиция вне диапазона start/stop токенов

Независимо от причины, orElseThrow() не должен падать в runtime — graceful handling через Optional защищает от любого из этих сценариев. Можно добавить логирование, чтобы понять первопричину в проде.

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.

NoSuchElementException: No value present

2 participants