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

[native_assets_builder] Concurrent execution of NativeAssetsBuilder #1319

Closed
3 tasks
dcharkes opened this issue Jul 11, 2024 · 1 comment · Fixed by #1384
Closed
3 tasks

[native_assets_builder] Concurrent execution of NativeAssetsBuilder #1319

dcharkes opened this issue Jul 11, 2024 · 1 comment · Fixed by #1384
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_assets_builder

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Jul 11, 2024

Currently, NativeAssetsBuilders methods can be called by the embedder as follows:

  • Concurrently running methods that work on different BuildConfig/LinkConfigs is fine. Everything is happening in the .dart_tool/native_assets_builder/<hashed_config>/.
  • Concurrently running methods on the same config is not supported.

This is currently mostly fine.

The situation in which this breaks in Flutter is when flutter run -d all has overlapping devices. E.g. if there are two Android phones connected. (It works fine with MacOS, iOS Simuatlor, iOS device, and Android emulator together.)

Flutter does not allow running more than one flutter command in the terminal concurrently. It has some kind of lock.

Dart doesn't have any commands running multiple native asset builds.

However, Dart doesn't guard against concurrent invocations. So two concurrent dart run process runs will cause undefined behavior.

Also there's no guard against running dart and flutter concurrently.

We should explore making the NativeAssetsBuilder re-entrant. It should support the following scenarios:

  • multiple invocations in the same Dart process Not needed.
  • multiple invocations in different Dart processes
  • it should keep working if any of these processes is abruptly terminated

Since multiple processes need to communicate, we'd likely want to use lock files on the file system.

dart:io surfaces locks: https://api.dart.dev/stable/3.4.4/dart-io/RandomAccessFile/lock.html

@dcharkes dcharkes added P2 A bug or feature request we're likely to work on package:native_assets_builder labels Jul 11, 2024
@dcharkes
Copy link
Collaborator Author

Due to flutter run -d all running native assets builds concurrently, we cannot share the hook compilation to kernel across the same hook but different config invocations in #1256.

@dcharkes dcharkes added 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 Jul 30, 2024
@dcharkes dcharkes changed the title [native_assets_builder] Concurrent execution of NativeAssetsBuilder (reentrancy) [native_assets_builder] Concurrent execution of NativeAssetsBuilder Jul 31, 2024
auto-submit bot pushed a commit that referenced this issue Aug 6, 2024
This makes the native assets builder lock the build directories so that it can be invoked concurrently from multiple `dart` and `flutter` processes.

Closes: #1319

The implementation of the locking is in `package:native_assets_cli/locking.dart` contains `runUnderDirectoryLock`, so that we can reuse it for #1379. (#1379 Requires more changes though, it needs to also give users access to a shared directory via the `BuildConfig`.)
HosseinYousefi pushed a commit that referenced this issue Aug 19, 2024
This makes the native assets builder lock the build directories so that it can be invoked concurrently from multiple `dart` and `flutter` processes.

Closes: #1319

The implementation of the locking is in `package:native_assets_cli/locking.dart` contains `runUnderDirectoryLock`, so that we can reuse it for #1379. (#1379 Requires more changes though, it needs to also give users access to a shared directory via the `BuildConfig`.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_assets_builder
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant