sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Directory.listSync sometimes not returning files

Open gmackall opened this issue 2 years ago • 11 comments

Context: https://github.com/flutter/flutter/issues/139378

We have an integration test in the flutter/packages repo that creates a file, reads it, and then checks also that the directory contains the file (actual contents of the test here):

  final File file = File('${directory.path}/$name');
  ...
  file.writeAsStringSync('Hello world!');
  expect(file.readAsStringSync(), 'Hello world!');
  expect(directory.listSync(), isNotEmpty);

This test is flaking from time to time on the last expect. I don't believe that this is a timing issue (it shouldn't be because these are synchronous operations, but also I've tried adding a sleep just in case and still saw it flake) nor a permissions issue (the read for the file still succeeds, and also it flakes even locally on my machine where the permissions aren't changing).

I've also tried looping for some time and comparing the output of directory.listSync to Process.runSync('ls', [directory.path]), and even when the file is missing in the former it is still there in the output of the latter.

Is there something that I'm missing that is wrong with the test here, or might there be an issue with Directory.listSync itself?


If you aren't sure, file the issue here, and we'll find the right home for it. In your issue, please include:

  • Dart version and tooling diagnostic info (dart info)
If providing this information as part of reporting a bug, please review the information
below to ensure it only contains things you're comfortable posting publicly.

#### General info

- Dart 3.3.0-198.0.dev (dev) (Wed Dec 6 04:03:18 2023 -0800) on "macos_arm64"
- on macos / Version 14.1 (Build 23B74)
- locale is en

#### Process info

| Memory |  CPU | Elapsed time | Command line                                                                               |
| -----: | ---: | -----------: | ------------------------------------------------------------------------------------------ |
|  22 MB | 0.0% |  01-18:08:37 | dart language-server --client-id=Android-Studio --client-version=AI-222.4459.24 --protocol=analyzer |
|  14 MB | 0.0% |  01-18:08:36 | dart language-server --client-id=Android-Studio --client-version=AI-222.4459.24 --protocol=analyzer |
|  10 MB | 0.0% |  02-23:10:18 | dart language-server --client-id=Android-Studio --client-version=AI-222.4459.24 --protocol=analyzer |
|  20 MB | 0.2% |  01-18:08:39 | flutter_tools.snapshot daemon                                                              |
|  26 MB | 0.1% |     19:54:16 | flutter_tools.snapshot daemon                                                              |
|  38 MB | 0.3% |        24:11 | flutter_tools.snapshot daemon 
  • Whether you are using Windows, macOS, or Linux (if applicable) Locally macOS, but flake is also happens on linux in CI.

Missing some or all of the above might make the issue take longer or be impossible to act on.

If you simply have a question, please consider using the listed Dart developer communities on the following page:

https://dart.dev/community#join-the-conversation

gmackall avatar Dec 08 '23 19:12 gmackall

cc @brianquinlan

mraleph avatar Dec 11 '23 13:12 mraleph

(this is most likely some Android specific wonkyness around permissions for external storage)

mraleph avatar Dec 11 '23 13:12 mraleph

I tried reproducing this with:

void _verifySampleFile(Directory? directory, String name) {
  if (directory == null) {
    return;
  }
  final File file = File('${directory.path}/$name');

  if (file.existsSync()) {
    file.deleteSync();
    assert(!file.existsSync());
  }

  file.writeAsStringSync('Hello world!');
  assert(file.readAsStringSync() == 'Hello world!');
  directory
      .listSync()
      .where((f) => FileSystemEntity.identicalSync(f.path, f.path))
      .first;
  file.deleteSync();
}

void test() async {
  for (var i = 0; i < 1000; ++i) {
    final List<StorageDirectory?> allDirs = <StorageDirectory?>[
      null,
      StorageDirectory.music,
      StorageDirectory.podcasts,
      StorageDirectory.ringtones,
      StorageDirectory.alarms,
      StorageDirectory.notifications,
      StorageDirectory.pictures,
      StorageDirectory.movies,
    ];

    for (final StorageDirectory? type in allDirs) {
      final List<Directory>? directories =
          await getExternalStorageDirectories(type: type);

      for (final Directory result in directories!) {
        _verifySampleFile(result, '$type');
      }
    }
  }
}

And ran this several time (each run tests the file creation 1,000 times) on the Android emulator, using various Android versions and could not reproduce.

Is this happening consistently? What happens if you change your test code like:

void _verifySampleFile(Directory? directory, String name) {
  expect(directory, isNotNull);
  if (directory == null) {
    return;
  }

  final File file = File('${directory.path}/$name');

  if (file.existsSync()) {
    file.deleteSync();
    expect(file.existsSync(), isFalse);
  }

  file.writeAsStringSync('Hello world!');
  expect(file.readAsStringSync(), 'Hello world!');
- expect(directory.listSync(), isNotEmpty);
+ expect(file.existsSync(), isTrue);
  file.deleteSync();
}

?

brianquinlan avatar Dec 13 '23 00:12 brianquinlan

Marking this as P2 because there is a workaround on the Flutter side.

brianquinlan avatar Dec 13 '23 00:12 brianquinlan

Is this happening consistently?

It is a relatively common flake, maybe 1 in 5 runs of that test shard?

What happens if you change your test code like:

void _verifySampleFile(Directory? directory, String name) {
  expect(directory, isNotNull);
  if (directory == null) {
    return;
  }

  final File file = File('${directory.path}/$name');

  if (file.existsSync()) {
    file.deleteSync();
    expect(file.existsSync(), isFalse);
  }

  file.writeAsStringSync('Hello world!');
  expect(file.readAsStringSync(), 'Hello world!');
- expect(directory.listSync(), isNotEmpty);
+ expect(file.existsSync(), isTrue);
  file.deleteSync();
}

?

I looped the tests with this modification 60 times and didn't see any flakes (did the same without the change and saw 9). I was just lazily using bash to loop so it was kind of slow, I can modify the tests to loop more times faster if we want more data

gmackall avatar Dec 13 '23 20:12 gmackall

Hmmm.... that's interesting so the file exists but listSync doesn't see it.

Is it possible that there is some sort of timing issue? If you delay one second before calling listSync then do you see this issue?

brianquinlan avatar Dec 14 '23 17:12 brianquinlan

Also, what's the intent of the test?

brianquinlan avatar Dec 14 '23 17:12 brianquinlan

Hmmm.... that's interesting so the file exists but listSync doesn't see it.

Is it possible that there is some sort of timing issue? If you delay one second before calling listSync then do you see this issue?

I've tried adding a one second delay before calling listSync (after creating the file and reading its contents) and still was able to reproduce the flake, so it doesn't seem like a timing issue to me.

The test is a test for the path_provider plugin, and is just testing that for each type of external storage directory we are able to get a directory, and that we can actually create a file in that directory (the function it is testing is here).

gmackall avatar Dec 18 '23 18:12 gmackall

The test is a test for the path_provider plugin, and is just testing that for each type of external storage directory we are able to get a directory, and that we can actually create a file in that directory (the function it is testing is here).

Then using expect(file.existsSync(), isTrue) would capture the same intent, right?

Are there any details about the test environment that you can share? Android version? Device?

brianquinlan avatar Dec 18 '23 19:12 brianquinlan

Then using expect(file.existsSync(), isTrue) would capture the same intent, right?

Yes, and we've also already landed a work around to the flake in https://github.com/flutter/packages/pull/5628, so this isn't currently blocking ci in any way.

Are there any details about the test environment that you can share? Android version? Device?

The ci configuration is here: https://github.com/flutter/packages/blob/main/.ci.yaml#L378, notably its using an android 14 (api 34) emulator. It also reproduces for me on my mac with an android 14 emulator. I haven't been able to reproduce on an android 13 emulator

gmackall avatar Dec 18 '23 19:12 gmackall

I ran into this - still happens on API 35 in every emulator I have tried. Happens ~0.15% of the time, just returns an empty list. You can read more details and code to replicate in the linked bug.

in the meantime everywhere we use listSync we do this:

      var ls = dir.listSync(recursive: false);
      // might be incorrect - check again
      if (ls.isEmpty) {
        ls = dir.listSync(recursive: false);
      }

reimager avatar Oct 14 '24 20:10 reimager