Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dynamic failures in analysis server during dart fix #50553

Open
eernstg opened this issue Nov 25, 2022 · 8 comments
Open

Dynamic failures in analysis server during dart fix #50553

eernstg opened this issue Nov 25, 2022 · 8 comments
Labels
analyzer-crash-report Issues which have been reported due to an analysis server crash area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on

Comments

@eernstg
Copy link
Member

eernstg commented Nov 25, 2022

I hope this is useful, even though I haven't made any attempts to create a minimal scenario. I've experienced a number of dynamic errors raised in the analysis server (2.19.0-421.0.dev) as follows. Run this command:

> cd $SDK/tests/language
> dart fix --dry-run .

The current commit of the SDK repo was e7985b28e9007bc4bfc9f1e5084c1b5376617fe8 .

One example run provided this output:

Computing fixes in language (dry run)...
Error from the analysis server: Exception while getting bulk fixes: type 'BooleanLiteralImpl' is not a subtype of type '_ParenthesizedCondition' in type cast
#0      AstBuilder.endSwitchStatement (package:analyzer/src/fasta/ast_builder.dart:3051:27)
#1      Parser.parseSwitchStatement (package:_fe_analyzer_shared/src/parser/parser_impl.dart:8278:14)
#2      Parser.parseStatementX (package:_fe_analyzer_shared/src/parser/parser_impl.dart:5250:14)
#3      Parser.parseStatement (package:_fe_analyzer_shared/src/parser/parser_impl.dart:5198:20)
#4      Parser.parseFunctionBody (package:_fe_analyzer_shared/src/parser/parser_impl.dart:5103:15)
#5      Parser.parseTopLevelMethod (package:_fe_analyzer_shared/src/parser/parser_impl.dart:3573:13)
#6      Parser.parseTopLevelMemberImpl (package:_fe_analyzer_shared/src/parser/parser_impl.dart:3318:14)
#7      Parser.parseTopLevelDeclarationImpl (package:_fe_analyzer_shared/src/parser/parser_impl.dart:574:14)
#8      Parser.parseUnit (package:_fe_analyzer_shared/src/parser/parser_impl.dart:392:15)
#9      Parser.parseCompilationUnit2 (package:analyzer/src/generated/parser.dart:107:32)
#10     Parser.parseCompilationUnit (package:analyzer/src/generated/parser.dart:103:12)
#11     FileState._parse (package:analyzer/src/dart/analysis/file_state.dart:709:23)
#12     FileState.parse (package:analyzer/src/dart/analysis/file_state.dart:489:14)
#13     FileState._getUnlinkedUnit (package:analyzer/src/dart/analysis/file_state.dart:655:16)
#14     FileState.refresh (package:analyzer/src/dart/analysis/file_state.dart:526:27)
#15     FileSystemState._newFile (package:analyzer/src/dart/analysis/file_state.dart:1435:10)
#16     FileSystemState.getFileForPath2 (package:analyzer/src/dart/analysis/file_state.dart:1267:14)
#17     FileSystemState.getFileForPath (package:analyzer/src/dart/analysis/file_state.dart:1251:12)
#18     AnalysisDriver.getResolvedLibrary (package:analyzer/src/dart/analysis/driver.dart:873:27)
#19     AnalysisSessionImpl.getResolvedLibrary (package:analyzer/src/dart/analysis/session.dart:103:26)
#20     BulkFixProcessor.fixErrors (package:analysis_server/src/services/correction/bulk_fix_processor.dart:244:52)
<asynchronous suspension>
#21     EditBulkFixes.handle (package:analysis_server/src/handler/legacy/edit_bulk_fixes.dart:43:20)
<asynchronous suspension>


#0      FileState.parse (package:analyzer/src/dart/analysis/file_state.dart:491:7)
#1      FileState._getUnlinkedUnit (package:analyzer/src/dart/analysis/file_state.dart:655:16)
#2      FileState.refresh (package:analyzer/src/dart/analysis/file_state.dart:526:27)
#3      FileSystemState._newFile (package:analyzer/src/dart/analysis/file_state.dart:1435:10)
#4      FileSystemState.getFileForPath2 (package:analyzer/src/dart/analysis/file_state.dart:1267:14)
#5      FileSystemState.getFileForPath (package:analyzer/src/dart/analysis/file_state.dart:1251:12)
#6      AnalysisDriver.getResolvedLibrary (package:analyzer/src/dart/analysis/driver.dart:873:27)
#7      AnalysisSessionImpl.getResolvedLibrary (package:analyzer/src/dart/analysis/session.dart:103:26)
#8      BulkFixProcessor.fixErrors (package:analysis_server/src/services/correction/bulk_fix_processor.dart:244:52)
<asynchronous suspension>
#9      EditBulkFixes.handle (package:analysis_server/src/handler/legacy/edit_bulk_fixes.dart:43:20)
<asynchronous suspension>


#0      FileState.parse (package:analyzer/src/dart/analysis/file_state.dart:491:7)
#1      FileState._getUnlinkedUnit (package:analyzer/src/dart/analysis/file_state.dart:655:16)
#2      FileState.refresh (package:analyzer/src/dart/analysis/file_state.dart:526:27)
#3      FileSystemState._newFile (package:analyzer/src/dart/analysis/file_state.dart:1435:10)
#4      FileSystemState.getFileForPath2 (package:analyzer/src/dart/analysis/file_state.dart:1267:14)
#5      FileSystemState.getFileForPath (package:analyzer/src/dart/analysis/file_state.dart:1251:12)
#6      AnalysisDriver.getResolvedLibrary (package:analyzer/src/dart/analysis/driver.dart:873:27)
#7      AnalysisSessionImpl.getResolvedLibrary (package:analyzer/src/dart/analysis/session.dart:103:26)
#8      BulkFixProcessor.fixErrors (package:analysis_server/src/services/correction/bulk_fix_processor.dart:244:52)
<asynchronous suspension>
#9      EditBulkFixes.handle (package:analysis_server/src/handler/legacy/edit_bulk_fixes.dart:43:20)
<asynchronous suspension>

The process was stuck at this point (no changes for about half an hour):

> sudo strace -p 160073
strace: Process 160073 attached
futex(0x7fff97b3b4f0, FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, 0, NULL, FUTEX_BITSET_MATCH_ANY) = ? ERESTARTSYS (To be restarted if SA_RESTART is set)

It seems like this procedure will yield many different crashes, perhaps because it will process the input libraries in a different order from time to time. Here is another one:

Computing fixes in language (dry run)...
Error from the analysis server: Exception while getting bulk fixes: Stack Overflow
#0      SubtypeHelper.isSubtypeOf (package:analyzer/src/dart/element/subtype.dart:36:3)
#1      SubtypeHelper.isSubtypeOf (package:analyzer/src/dart/element/subtype.dart:165:11)
#2      SubtypeHelper.isSubtypeOf (package:analyzer/src/dart/element/subtype.dart:285:25)
#3      SubtypeHelper.isSubtypeOf (package:analyzer/src/dart/element/subtype.dart:165:11)
#4      SubtypeHelper.isSubtypeOf (package:analyzer/src/dart/element/subtype.dart:285:25)
#5      SubtypeHelper.isSubtypeOf (package:analyzer/src/dart/element/subtype.dart:165:11)
#6      SubtypeHelper.isSubtypeOf (package:analyzer/src/dart/element/subtype.dart:285:25)
#7      SubtypeHelper.isSubtypeOf (package:analyzer/src/dart/element/subtype.dart:165:11)
...
@eernstg eernstg added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-crash-report Issues which have been reported due to an analysis server crash labels Nov 25, 2022
@srawlins
Copy link
Member

CC @scheglov recent changes in switches?

@srawlins srawlins added P2 A bug or feature request we're likely to work on P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Nov 28, 2022
@bwilkerson
Copy link
Member

bwilkerson commented Nov 28, 2022 •
edited by srawlins

Sounds like there are at least three bugs here:

  • the unguarded cast in AstBuilder
  • the infinite loop in SubtypeHelper
  • the hang in dart fix when the server fails to send a response to the request

@scheglov
Copy link
Contributor

  1. The infinite loop in SubtypeHelper has always been there, last time when I looked, I decided that it works as specified, i.e. there is a loop in the spec.

  2. I can reproduce the crash with this code:

// @dart = 2.18
void f(Object value) {
  switch (value) {
    case (int a,) when a == 0:
  }
}

It happens because in endCaseExpression we pop components of SwitchPatternCaseImpl only when _featureSet.isEnabled(Feature.patterns), but the parser always pushes "something" for the pattern and when clauses. So, we actually pop the guard expression, and leave the "pattern" (not really, actually ParenthesizedExpression because the parser does not create patterns when not enabled, but still creates something) in the stack.

@bwilkerson
Copy link
Member

... I decided that it works as specified, i.e. there is a loop in the spec.

I don't think it's reasonable to say that an infinite loop in the spec means that it's ok for server to crash with a stack overflow. Can we please get that fixed?

@eernstg for the bug in the spec.

@scheglov
Copy link
Contributor

scheglov commented Nov 28, 2022 •
edited

See #46524 which I opened a while ago for tracking.
I updated it with a simple reproduction code.

@scheglov
Copy link
Contributor

https://dart-review.googlesource.com/c/sdk/+/272387 should fix the crash for switch.

copybara-service bot pushed a commit that referenced this issue Nov 29, 2022
@scheglov
Fix parsing switch statement with SwitchPatternCaseImpl when the patt…
2b5608d 
…erns feature is disabled.

Bug: #50553
Change-Id: Icd87d422ebf899d4a0d56ab4b7e476ab60949ad4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272387
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
@srawlins
Copy link
Member

I think this is still P1 because an infinite loop can still occur just by running the simple dart fix command @eernstg provided, in the tests/language directory. But if that's... weird or no one does it or no user has ever reported it, or it's acceptable for a few weeks until we get a change to the language spec, maybe it can be downgraded?

@srawlins
Copy link
Member

srawlins commented Feb 8, 2023

I'm changing my mind on this one. The infinite loop has been around for ages, and we've only had one user report (internally, also from the language tests directory). I think this is just P2.

@srawlins srawlins added P2 A bug or feature request we're likely to work on and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
analyzer-crash-report Issues which have been reported due to an analysis server crash area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@srawlins @scheglov @bwilkerson @eernstg