Cronet JNI crash in release and profile builds #1241
Comments
lukehutch
added
package:cronet_http
type-bug
|
Just a thought -- could the race condition be caused by the fact that I am not Future<void> main() async {
// Set up Cronet http client on Android or Cupertino http client on iOS
if (Platform.isAndroid) {
final engine =
CronetEngine.build(cacheMode: CacheMode.memory, cacheMaxSize: 1000000);
await runWithClient(run, () => CronetClient.fromCronetEngine(engine));
} else if (Platform.isIOS || Platform.isMacOS) {
final config = URLSessionConfiguration.ephemeralSessionConfiguration()
..cache = URLCache.withCapacity(memoryCapacity: 1000000);
await runWithClient(run, () => CupertinoClient.fromSessionConfiguration(config));
} else {
// Run with the default IOClient
await run();
}
}
Future<void> run() async {
await runApp(const MyApp());
} |
|
I can confirm that adding Also, the crash happens only on the emulator (API 34, Pixel 8 Pro), not on my physical Pixel 7 Pro. I don't know whether fixing this lockup will also fix the crash that I was getting with my full app when uploading release builds to Google Play, or when running the app in profile mode locally, but I'll keep trying to reproduce that. |
I'm not an http expert but based on the docs I don't think that you can call this before /// When used in the context of Flutter, [runWithClient] should be called before
/// [`WidgetsFlutterBinding.ensureInitialized`](https://api.flutter.dev/flutter/widgets/WidgetsFlutterBinding/ensureInitialized.html)
/// because Flutter runs in whatever [Zone] was current at the time that the
/// bindings were initialized. |
|
I don't know what the root cause is but getting Flutter applications to work with |
|
Eureka! Maybe... I think I have found the problem with this whole thing: I suggest not using |
Correct, I forgot I had seen this in the docs. In my full app, I run So that is probably the source of the segfault I was seeing on Google Play, if it means that the JNI context would not be initialized before cronet calls are made. I will try the example from the docs pointed to by @brianquinlan . However if I have learned anything from tinkering with this, almost any change can resolve the crash -- there is some raciness here. |
This is due to the use of zones. I had not go into the source of One way to fix this problem while still using if (Platform.isAndroid) {
await runWithClient(
run,
() {
final engine =
CronetEngine.build(cacheMode: CacheMode.memory, cacheMaxSize: 1000000);
return CronetClient.fromCronetEngine(engine);
},
);
} |
|
The The I don't remember where I got the |
How does this ensure initialization has already happened? I think the main point here is that you moved the engine creation inside Should I still be wary of doing things this way, for the reasons @brianquinlan suggested, that
|
|
I started to port my code to use You can provide a So basically I really do need to get this working properly with |
The method that you pass to
@internal
Client? get zoneClient {
final client = Zone.current[#_clientToken];
return client == null ? null : (client as Client Function())();
}This means that the first time Assuming all of your http requests happen after
The reason I provided you with a fix for |
Thanks. But what is the consequence of using It used to be that Flutter would arbitrarily run things in the root zone, which would use the default HTTP client, not the configured client: #828 However, it looks like that was fixed. Are there still remaining issues? |
I don't know enough about how Flutter deals with zones or http to give you a definitive answer, but based on what I see I don't think there should be any problems with this approach as long as you use widgets that use http inside your widget tree. By definition, the widget tree will be created AFTER It actually does not matter in which zone the callback is called, as long as we had never called An open question is what about any changes to this context? Should the cronet engine get rebuilt after the context changes as well? What about background http tasks? Do we have access to the same context in the background as well? If so, then the cronet engine has to listen to context changes instead of using |
|
Just for the record, there is one other confirmation of the JNI crash here: Baseflow/flutter_cached_network_image#649 (comment) -- so it's not just my app running on my system. But they saw this only once in their app. |
I see, I asked to know more about the way they do it. I'm personally not happy with a blanket "oh this sometimes doesn't work, so let's not use it", I want to know why it doesn't. It might very well be an issue with the fact that cronet does not "update" the context for example. |
Right, exactly -- I have updated my code to not use |
|
ok, I could just reproduce this, it has nothing to do with runWithClient if you use an Android without play services cronnet seems not be available. Easiest fix to prevent this crash: const _maxCacheSize = 2 * 1024 * 1024;
Client httpClientFactory() {
try {
if (Platform.isAndroid) {
final engine = CronetEngine.build(
cacheMode: CacheMode.memory,
cacheMaxSize: _maxCacheSize,
enableHttp2: true,
);
return CronetClient.fromCronetEngine(engine);
}
/// 'You must provide the Content-Length' HTTP header issue
if (Platform.isIOS || Platform.isMacOS) {
final config = URLSessionConfiguration.ephemeralSessionConfiguration()
..cache = URLCache.withCapacity(memoryCapacity: _maxCacheSize);
return CupertinoClient.fromSessionConfiguration(config);
}
} catch (_) {
return IOClient(HttpClient());
}
var httpClient = HttpClient();
// To use with Fiddler uncomment the following lines and set the
// ip address of the machine where Fiddler is running
// httpClient.findProxy = (uri) => 'PROXY 192.168.1.61:8866';
// httpClient.badCertificateCallback =
// (X509Certificate cert, String host, int port) => true;
return IOClient(httpClient);
} |
|
@brianquinlan propbably this precaution should be added to the example and the HTTP docs |
|
@escamoteur The crash I was seeing was on Google Experience / Google Play Services devices (Pixel Pro 7 physical device, and Pixel Pro 8 emulator). But thanks for pointing out the need for |
|
@lukehutch actually your crash on top here seems to be a different one than the JNI one as its from the NDK |
|
I got this here in that case: |
|
@lukehutch how do you link the NDK stack trace to the JNI call? This is what we do to setup the cacheManager const cacheKey = 'watchcrunch_cache';
di.registerSingleton(Client());
final appState = di<AppState>();
di.registerSingleton<CacheManager>(
WcImageCacheManager(
Config(
cacheKey,
fileService: WcHttpFileService(
httpClient: di<Client>(),
),
stalePeriod: appState.imageDiscStalePeriod,
maxNrOfCacheObjects: appState.imageDiscCacheSize,
),
),and we do the runWithClient directly in our main and we don't await it. Not problem so far with our App that we released yesterday with the new Clients. only the one without Play Services |
The top line of the stacktrace is in
So far, no. I am not using It sounds like @HosseinYousefi has a good understanding of what was causing the crash. The solution might just be better documentation, but I would hope there is a way to more robustly protect against JNI being uninitialized when it's needed. The open questions in #1241 (comment) will hopefully be answered before this is closed. |
|
hmm, what I don't understand is why it works without any problems for us. cronet_http: ^1.3.1 dependency_overrides: |
|
If Cronet is not available then you should get a clear The |
|
The downside is that cronnet is bundled with the apk in that case which increases your app's size. It doesn't seem possible to download it automatically when it isn't bundled or play services are not available.
@brianquinlan it probably would be helpful if the
exception would mention the dependency on Google Play Services and/or the docs on the configuration of the http package would contain a sentence mentioning it (the table about which platform supports which clients just states Android)
Also the given example code should contain a try catch to handle the situation that no cronnet is available.
Am 29. Juni 2024, 04:42 +0100 schrieb Luke Hutchison ***@***.***>:
… > The package:cronet_http documentation explains how to get the package to work if the phone does not include Google Play Services.
Thanks for the tip, Brian. However, is there no way to do this without using a commandline switch? Shouldn't it be possible to depend upon org.chromium.net:cronet-embedded directly? And/or maybe a package cronet_http_embedded could be created that only depends on the embedded version?
Are there any downsides to using the embedded version? If not, what benefit is obtained by requiring Play Services for the non-embedded version?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
This should not be any different than using cronet on Android without Flutter. How do they solve this? This beginner codelab seems to have the answer: https://developer.android.com/codelabs/cronet#4 They use |
|
@HosseinYousefi if that is possible with flutter it should be the way to go. Ideally the package should take care of this on his own. |
|
Why can't pre-built libraries be shipped with the Flutter package, like with every other NDK library? Is there a good reason the package has to be fetched from Google Play? |
I don't see why it shouldn't be possible. Feel free to make a PR
You can ship it. However there are some benefits that you gain if you use play services. From https://developer.android.com/codelabs/cronet#3:
Simply follow the instructions here: https://github.com/dart-lang/http/tree/master/pkgs/cronet_http#use-embedded-cronet |
|
Yeah, 5MB is quite a lot. Not sure how much is saved if you select to exclude the default HttpClient implementation.
Downloading on demand would be the best.
Unfortunately I m no Android native dev, so someine else would have to make that PR
Am 30. Juni 2024, 11:53 +0100 schrieb Hossein Yousefi ***@***.***>:
… > @HosseinYousefi if that is possible with flutter it should be the way to go. Ideally the package should take care of this on his own.
I don't see why it shouldn't be possible. Feel free to make a PR
> Why can't pre-built libraries be shipped with the Flutter package, like with every other NDK library? Is there a good reason the package has to be fetched from Google Play?
You can ship it. However there are some benefits that you gain if you use play services. From https://developer.android.com/codelabs/cronet#3:
> The Cronet team recommends using the Google Play Services provider. By using the Google Play Services provider, your application doesn't need to pay the binary size cost of carrying Cronet (about 5 megabytes) and the platform ensures that latest updates and security fixes are delivered.
Simply follow the instructions here: https://github.com/dart-lang/http/tree/master/pkgs/cronet_http#use-embedded-cronet
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Yeah, I didn't mean you specifically – However we're making it really easy to develop these things using JNIgen. Anyone implementing this wouldn't have to write any Java or Kotlin code. Everything can be done using the generated Dart bindings. |
Could we get a cronet_http_embedded package on pub.dev though, please, for developers who just want to ship it with their app? For Play experience devices, the total download size is the same. I'm not worried about adding another 5MB when tons of apps today have download sizes over 100MB. |
I think that this will only work if the device has Play Services installed. The last line says:
|
Maintaining another package was a significant amount of work, which is why we switched to using a configuration flag. Could you explain why the configuration flag doesn't work for you? |
I couldn't understand how to use this. Is this a runtime flag, or a compiletime flag? The examples are for It's all good though, I changed my code to just fall back to The things I would still love to see are:
I had another weird lockup issue on startup ( flutter/flutter#147967 ), and I think it was also due to the JNI initialization issue, because as far as I can tell (it was sporadic), it went away when I stopped using |
It's a compile time flag. You can use that same flag with the |
From https://developer.android.com/codelabs/cronet#3:
Also the dependencies should change to include |
|
My point was that you still need Google Play Services. If you don't have Google Play Services installed (e.g. you need to support a phone distributed in China) then this approach won't work. |
|
OK, thanks Brian and Houssein for explaining the Google Play situation. I'll leave this issue open for the other JNI-related stuff (I believe it is still important to avoid crashes and lockups even if developers try to initialize Cronet before Flutter -- and the docs need improving). |
The zone machinery works as long as one uses it as it was designed: If a library accepts callback functions, the library should bind those callback functions to the current zone, ensuring that once the callbacks are invoked (caller can be from another zone) it switches to the target zone (where the callback was bound to) before executing the callback. In pure Dart this works, because Dart's core libraries ensure callbacks are bound to the current zone (e.g. import 'dart:async';
void main() {
late final Zone fromZone;
late final Zone toZone;
late final Stream stream;
runZoned(() async {
fromZone = Zone.current;
final controller = StreamController();
stream = controller.stream;
for (int i = 0; i < 10; ++i) {
await Future.delayed(const Duration(milliseconds: 1));
controller.add(i);
}
});
runZoned(() async {
toZone = Zone.current;
stream.listen((data) { // <-- this will use `Zone.current.bindUnaryCallback()` internally
print('Adding zone is different from consuming zone: ${!identical(fromZone, toZone)}');
print(data);
});
});
}Here the callback given to => This only works because Many of flutter's APIs do not do that, for example taking the default class _MyHomePageState extends State<MyHomePage> {
int _counter = 0;
+ late Zone expectedZone;
void _incrementCounter() {
+ print('Button pressed callback in correct zone: ${identical(expectedZone, Zone.current)}');
+ print('Button pressed callback in run in root zone: ${identical(Zone.root, Zone.current)}');
setState(() { _counter++; });
}
@override
Widget build(BuildContext context) {
+ return runZoned(() {
+ expectedZone = Zone.current;
return ... (
floatingActionButton: FloatingActionButton(
onPressed: _incrementCounter,
tooltip: 'Increment',
child: const Icon(Icons.add),
...);
+ });
}
}This will print => The callback given to We can "fix" this manually via floatingActionButton: FloatingActionButton(
- _incrementCounter,
+ onPressed: Zone.current.bindCallback(_incrementCounter),
...);=> So the root cause is flutter's choice of not binding callbacks obtained from APIs to the current zone. Flutter under the hood has a C++ engine, that will decide when frames are to be drawn and explicitly call into Dart to render frames. It has to choose what zone those calls should run inside of. => I believe it will use the zone that was active when the bindings were initialized. For every frame to be drawn flutter C++ engine will call _drawFrame @pragma('vm:entry-point')
void _drawFrame() {
PlatformDispatcher.instance._drawFrame();
}which calls PlatformDispatcher.instance._drawFrame // Called from the engine, via hooks.dart
void _drawFrame() {
_invoke(onDrawFrame, _onDrawFrameZone);
}which will call => That zone is the one that was used when initializing the bindings in flutter, which either happens implicitly in |
@HosseinYousefi Two questions: |
Android context can be nullable. Also there might be a need for listening to context changes, it's good to expose some APIs that enable this.
Yes, we're relying on the existing Flutter plugin system. I will explore this when working on " Maybe cronet specifically doesn't need the context from Flutter plugin system and can instead use the global context. That one is easy to get: |
I think that using |
|
actually we use runWithClient in our app without any problems with Fluter. I'm pretty sure the reason for this sort of crash is something else |
|
@lukehutch are you running on 32-bit Android? If so, there was a bug fixed in |
I identified the reason. It was the fact that context was not initialized. You can find my answer above. I already mentioned the 32 bit bug to Luke. |
|
@brianquinlan We can safely close this issue. Here's why the crash happened: #1241 (comment) |
|
Do you think that it is worth mentioning in the docs that either (and sorry, I lost track of the state of this issue ;-)) |
Yes, that's helpful to mention. I have opened two issues to 1. Output a descriptive error message if users do this. 2. See if I can stop depending on flutter bindings for retrieving the right context needed by cronet. I'll open PRs here to upgrade the dependencies once that's done as usual! |
|
Do you think that work will be done in the short term? I don't want to add documentation if I'll end it removing it (assuming that I don't forget!) in a month. |
I'll prioritize no. 1, but that's in line with the documentation. I don't know even if no. 2 is possible right now. I have to check. |
I am getting an NDK crash in
libdartjni.so (GetApplicationContext+60)when I upload a release build of my APK to the Play Store. (The debug build runs fine locally.)I eventually ran into this additional issue of profile builds crashing locally (Android emulator, API level 34), which I then figured out had the same cause:
flutter/flutter#149229
This is a JNI crash, so I filed this JNI bug: dart-lang/native#1150 However, @HosseinYousefi asked me to also file a Cronet bug, because that is the only project I am using that makes JNI calls.
Here are the Cronet setup steps I am using:
pubscec.yaml:main.dart:I created this project as an attempt at a minimal reproducer, which adds only the above to the default Flutter project:
https://github.com/lukehutch/cronetcrash
I cannot reproduce the crash using this project, but it causes a different issue: release builds freeze on startup, before the UI is fully drawn (debug and profile builds start up fully):
Here is the logcat from the app startup when the release build freezes:
logcat output
After the app has frozen on startup, I can't attach the debugger in Android Studio to it (there is nothing shown to attach to). The pause button in the VS Code debugger does nothing.
Here is the logcat that is shown when killing the app from the VS Code debugger after the app is frozen:
logcat output
Here is the output of
flutter doctor -v:`flutter doctor -v`
The text was updated successfully, but these errors were encountered: