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.

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

dart format hangs on Windows when reading from stdin in the presubmit #46947

Open
athomas opened this issue Aug 19, 2021 · 6 comments
Open

dart format hangs on Windows when reading from stdin in the presubmit #46947

athomas opened this issue Aug 19, 2021 · 6 comments
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool.

Comments

@athomas
Copy link
Member

athomas commented Aug 19, 2021

The script below hangs on Windows but works fine on other platforms (change the hard coded depot_tools location in the script before running it):

#!/usr/bin/env python3

import imp
import os
import subprocess
import sys

depot_tools = '/Users/user/src/depot_tools'
sys.path.append(depot_tools)
import scm

utils = imp.load_source('utils', os.path.join('tools', 'utils.py'))
dart = os.path.join(utils.CheckedInSdkPath(), 'bin', 'dart')

windows = utils.GuessOS() == 'win32'
if windows:
    dart += '.exe'

args = [
    dart,
    'format',
    '--set-exit-if-changed',
    '--output=none',
    '--summary=none',
]

print('Body builder from git')
contents = scm.GIT.Capture(['show', 'c9d954efe7c7a71d2bf2c2ca385855b82fbfb379:pkg/front_end/lib/src/fasta/kernel/body_builder.dart'],
                                strip_out=False)
process = subprocess.run(args, input=contents, text=True)
print(process.returncode)

For some reason, dart format hangs reports weird formatting errors and starts to print messages like this:

Attempt:11 waiting for isolate vm-isolate to check in
Attempt:11 waiting for isolate dartdev to check in

On other platforms, the script exits normally and dart format finds no formatting issues.

@bkonyi @munificent Any ideas about what could go wrong here? Perhaps line ending confusion?

/cc @johnniwinther

@athomas athomas added the area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. label Aug 19, 2021
@athomas
Copy link
Member Author

athomas commented Aug 19, 2021

Note that simpler test cases like these work even on Windows:

contents = """
void main()                     {}
"""
print('Format change')
process = subprocess.run(args, input=contents, text=True)
print(process.returncode)
 
contents = """
void main()                     {
"""
print('Syntax errror')
process = subprocess.run(args, input=contents, text=True)
print(process.returncode)
 
contents = """void main() {}
"""
print('No change')
process = subprocess.run(args, input=contents, text=True)
print(process.returncode)

@mraleph
Copy link
Member

mraleph commented Aug 19, 2021

Is this a duplicate of #46600 (though that one was marked as fixed).

@athomas
Copy link
Member Author

athomas commented Aug 19, 2021

I don't think so. Before #46600, this would hang on macOS as well. Now it seems to only happen on Windows for specific inputs.

Also, a part of the fix was 44ffad5 which may have masked the issue. It removed the process.communicate causing the presubmit to no longer block on the dart format call. That broke the formatting check. This was fixed in d2bd43f which re-enabled the formatting checks.

@athomas
Copy link
Member Author

athomas commented Aug 19, 2021

The checked-in SDK is currently at 2.14.0-377.4.beta, which has the dart_style fixes for #44600.

@johnniwinther
Copy link
Member

Any update on this? I am forced to upload most of my CLs with --bypass-hook because of this, and who knows how many badly formatted files have been committed due to this.

dart-bot pushed a commit that referenced this issue Sep 1, 2021
#46947

Change-Id: Ia632fbc73627504f96ef06fa99fff1f343ef6698
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212047
Reviewed-by: Johnni Winther <johnniwinther@google.com>
@munificent
Copy link
Member

munificent commented Sep 8, 2021

Sorry for the delay. I was out on vacation.

For some reason, dart format hangs reports weird formatting errors and starts to print messages like this:

Attempt:11 waiting for isolate vm-isolate to check in
Attempt:11 waiting for isolate dartdev to check in

I've never seen anything like that before, sorry. :( I also don't have a Windows machine to try to repro this on.

For what it's worth, dart_style does have tests for reading from stdin and I believe those tests are run and pass on Windows in the SDK. However, those tests also weren't sufficient to catch #46600. I don't recall exactly why. I think maybe because the tests spawn dart format not using a shell, which affects the behavior?

It looks like those messages are printed by the VM (DumpAliveIsolates()) when it is waiting for the isolates to shutdown? Maybe the main app isolate is hanging because it's waiting for more input on stdin or something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool.
Projects
None yet
Development

No branches or pull requests

4 participants