Add 300ms sleep before calling apk.start.

[jingchan]

Adding a short delay here appears to fix rust-mobile/cargo-apk#10 at least for my system.

First time contributing, so please let me know if there are other conventions I should follow.

Thanks!

[Jasper-Bekkers]

The commit seems empty, though I don't think adding a 300ms delay to fix a race condition is a proper way to fix these things.

[MarijnS95]

The app is started with am start -W which should guarantee that is has started? If that isn't the case we might want to spinloop on pidof until a given timeout - let's check how ndk-gdb does it instead of hand-rolling our own solution (300ms is after all very long and arbitrary).

[Jasper-Bekkers]

One thing that occurred to me - we don't strictly need the PID, it's just to output the logcat results. If we can wrangle logcat to filter based on activity name we might also be fine (though - we may also introduce problems if we have multiple of the same activities running).

[MarijnS95]

@Jasper-Bekkers It doesn't seem like the logcat buffer includes the package name (or activity name), there's only log tags which are application-defined and impossible for us to guess (set to RustStdoutStderr for the forwarding code in ndk-glue, and our own application uses module names through the log facade).

All apps run under a unique user (UID), and that's included in the logcat buffer. Our app tends to switch UIDs though (perhaps because it is being reinstalled, e.g. when switching debug signatures) but at least logcat --uid supports a comma-separated list. We just need a clean way to process pm dump rust.<appname>.

Alas, Google has already solved this problem for Android Studio - we just have to look up how they solve this problem instead of reinventing the wheel.

[jingchan]

Sorry for the empty commit- I believe it's resolved now. Thanks for being patient!

I'd agree a more proper solution is warranted here, and I'd be happy to contribute that! Though I'm somewhat unfamiliar with the rust (and android-rust) ecosystem, so I'd appreciate some guidance along the way.

For reference, this is how Google achieves this in their bazel tooling, which I would presume could be considered close to if not the "gold standard" for Android deployment (this is likely similar or the same as is done internally): https://github.com/bazelbuild/bazel/blob/f65a0d6a87619022de5a6a28ddd7a30ea3cf3186/tools/android/incremental_install.py#L782-L797

[MarijnS95]

@jingchan Are you sure this change change solves the problem? You inserted the timeout in between apk.install() and apk.start(), meaning that the Activity won't get launched (but the install might start up the process) before the timeout.

Instead, should the timeout reside in between am start -W and pidof? In rust-mobile/cargo-apk#10 you mention "Appears to be a race condition where apk.start is called before the system is ready to start the app and return a PID" yet the logs seem to indicate that the activity was started successfully - which is strange given that it should be running after that, thanks to -W.

Do you think the installation/app-replacement is still running in the background at that point, causing am start to start an older instance of the Activity which is killed pretty much immediately after, when the installation completes?

---

Alas, the Bazel build script doesn't seem to wait for the activity to start nor tries to acquire a PID, so it's different.

For testing, can you test if the Activity always launches correctly after commenting out the pidof and logcat invocations? I'm curious if the app doesn't start at all in your case (as this always worked before, AFAIK) or if it's merely an issue with the freshly-added pidof+logcat logic.
In other words, even if cargo apk run fails on Error: String `` is not a PID, is the app still launched?

[MarijnS95]

Closing for not seeing any followup on the questions and suggestions posed above. Besides, the cargo-apk portion of this repo migrated to https://github.com/rust-mobile/cargo-apk 🎉 - any followup should be submitted there.