-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
SecureSocket handshake can stall the main thread for long time #41519
Comments
We ran into the same issue. For us our the issue was caused by our "User-Agent overwrite" not working anymore with Flutter 1.12, which cause the server to respond with status 400. Maybe providing some user-agent header ( |
me too! |
url: https://gank.io/api/v2/categories/Article ios ui freeze on http request |
Same issue. It's really a very very very terrible problem. Relative issues: dart-lang/http#400. |
Does this replicate when you use The comments so far don't give much to investigate - is this because there is a Does anyone have a minimal reproduction? |
@natebosch In my case, i was using |
If the website uses lets encrypt certification, the ui will be freeze on https request. |
@vifird - the concern isn't having imports to @beiger - Can you get more specific? Is there a Future that should complete but doesn't? Does the entire runtime crash?
If you can reproduce without using this package we will need to route the issue to the Dart SDK repo. Do you have a complete and minimal reproduction case? |
@natebosch import 'dart:io';
import 'package:flutter/material.dart';
import 'package:package_info/package_info.dart';
class AboutPage extends StatefulWidget {
@override
AboutPageState createState() => AboutPageState();
}
class AboutPageState extends State<AboutPage> {
String _version = "";
double _top = -20;
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
title: "about",
),
body: Stack(
children: <Widget>[
AnimatedPositioned(
duration: const Duration(milliseconds: 2000),
curve: Curves.bounceOut,
left: 0,
top: _top,
child: Container(
width: MediaQuery.of(context).size.width,
child: Center(
child: Text(
_version,
style: TextStyle(
fontSize: 28,
color: Colors.blue,
fontWeight: FontWeight.bold
),
),
),
),
)
],
),
);
}
@override
void initState() {
super.initState();
_getVersion();
test();
}
Future<void> test() async {
// var url="https://gank.io/"; // Letsencrypt CA
var url='https://github.com/dart-lang/http/issues/374'; // other certificate
var httpClient = HttpClient();
var request = await httpClient.getUrl(Uri.parse(url));
var response = await request.close();
print(response);
}
void _getVersion() {
Future.delayed(Duration(microseconds: 500), () {
PackageInfo.fromPlatform().then((value) {
setState(() {
_version =value.version;
_top = MediaQuery.of(context).size.height / 2.7;
});
});
});
}
} When you open this page, sometimes the page gets stuck in the middle, sometimes the animation gets stuck. Other CA url: |
Have you tried using an https://api.dart.dev/stable/2.7.2/dart-io/HttpClient/badCertificateCallback.html |
I think I'll move to the SDK repo for now. It's not entirely clear to me what the issue is, but I don't see any evidence it's related to |
I'm using HttpClient from dart:io. I tried to set the badCertificateCallback, the program did not enter callback. I've only had this kind of stuck situation since last month, and there are some people who have met earlier than me. It used to run well with the same code several month ago. This is only on IOS, Android is good. |
I don't think there is enough information here for us to diagnose problem. HTTP processing is done asynchronously so it can't just block your UI thread. I would start by looking for exceptions in the logs. |
Many people have encountered this situation. |
I also have this problem, I tried the same request in Safari, It's also slow, Server ENV:
Client ENV:
TEST: My Flutter App:
iOS Safari:
MacOS 10.15.4 Safari (without cache):
|
@CyrilHu could you try running the following code on your iOS device (replace url to point to your https server) import 'dart:async';
import 'dart:developer' as developer;
import 'dart:io';
void log(String s) {
print(s);
developer.log(s);
}
Timer stallDetector() {
final sw = Stopwatch()..start();
return Timer.periodic(Duration(milliseconds: 5), (_) {
if (sw.elapsedMilliseconds > 10) {
log('EVENT LOOP WAS STALLED FOR ${sw.elapsedMilliseconds} ms');
}
sw.reset();
});
}
void main() async {
final url = 'https://gank.io';
final timer = stallDetector();
var sw = Stopwatch()..start();
var httpClient = HttpClient();
try {
var request = await httpClient.getUrl(Uri.parse(url));
var response = await request.close();
log('REQUEST COMPLETE IN ${sw.elapsedMilliseconds} ms');
} finally {
httpClient.close(force: true);
timer.cancel();
}
} what does it print back? |
https test 1: https test 2: http test 1: http test 2: |
@mraleph |
Yeah, this seems pretty bad. Based on my cursory analysis of the code most likely cause of the the problem is the fact that we install a certificate verification callback (via However
Which seems to match the situation we are experiencing here - I think the stall occurs when It is unclear if there are any work-arounds on the client side. I am extremely unfamiliar with this code e.g. it is unclear to me why we use @bkonyi @zanderso @dnfield you are the ones changing this code most. Can you take a look at this? |
@ghostgzt I think the only client side workaround I can offer is to use a separate isolate to handle networking and then send results back into the main isolate. This would prevent UI from stalling - but hand shake would still take very long time. |
It looks like there's a SecTrustEvaluateAsyncWithError that we might be able to use here instead. We were using |
It turns out that I actually meant // SSL_CTX_set_cert_verify_callback sets a custom callback to be called on
// certificate verification rather than |X509_verify_cert|. |store_ctx| contains
// the verification parameters. The callback should return one on success and
// zero on fatal error. It may use |X509_STORE_CTX_set_error| to set a
// verification result.
//
// The callback may use |SSL_get_ex_data_X509_STORE_CTX_idx| to recover the
// |SSL| object from |store_ctx|.
OPENSSL_EXPORT void SSL_CTX_set_cert_verify_callback(
SSL_CTX *ctx, int (*callback)(X509_STORE_CTX *store_ctx, void *arg),
void *arg); enum ssl_verify_result_t BORINGSSL_ENUM_INT {
ssl_verify_ok,
ssl_verify_invalid,
ssl_verify_retry,
};
// SSL_CTX_set_custom_verify configures certificate verification. |mode| is one
// of the |SSL_VERIFY_*| values defined above. |callback| performs the
// certificate verification.
//
// The callback may call |SSL_get0_peer_certificates| for the certificate chain
// to validate. The callback should return |ssl_verify_ok| if the certificate is
// valid. If the certificate is invalid, the callback should return
// |ssl_verify_invalid| and optionally set |*out_alert| to an alert to send to
// the peer. Some useful alerts include |SSL_AD_CERTIFICATE_EXPIRED|,
// |SSL_AD_CERTIFICATE_REVOKED|, |SSL_AD_UNKNOWN_CA|, |SSL_AD_BAD_CERTIFICATE|,
// |SSL_AD_CERTIFICATE_UNKNOWN|, and |SSL_AD_INTERNAL_ERROR|. See RFC 5246
// section 7.2.2 for their precise meanings. If unspecified,
// |SSL_AD_CERTIFICATE_UNKNOWN| will be sent by default.
//
// To verify a certificate asynchronously, the callback may return
// |ssl_verify_retry|. The handshake will then pause with |SSL_get_error|
// returning |SSL_ERROR_WANT_CERTIFICATE_VERIFY|.
OPENSSL_EXPORT void SSL_CTX_set_custom_verify(
SSL_CTX *ctx, int mode,
enum ssl_verify_result_t (*callback)(SSL *ssl, uint8_t *out_alert)); In
Apparently there is an |
@a-siva @franklinyow - is this a candidate for the Sept milestone? |
Team need more time for the fix, moved to Oct |
@a-siva Who's working on this now? |
@aam has picked this up. |
Per discussion with @rmacnak-google the plan is to leave existing synchronous openssl/boringssl API in place, but have SSLFilter::Handshake() run on dart worker thread using IOService infrastructure that is used for similar synchronous os api calls(file, socket, etc) |
One problem with running SSLFilter::Handshake on separate worker thread is its inability to invoke dart's |
|
Thanks @zanderso . Yeah, that is a possibility, concern would be clean up of blocked IOService thread in case As a temporary solution I thought of having async handshake only in case when there is no Yet another possibility is to use |
https://dart-review.googlesource.com/c/sdk/+/165520 with proposed fix. |
…on validation on separate worker thread. Running trust evaluation system api call on worker thread effectively unblocks main isolate when it attempts to establish https connection. Execution of the worker thread is implemented via dart native port infrastructure which allows to run C++ code as a dart message handler. TrustEvaluateHandlerFunc (if provided by platform-dependent SSLCertContext) is that handler, only mac(ios) implementation provides it. Asynchrony of CertificateVerificationCallback (for mac/ios) is implemented via [ssl_verify_retry] return code which allows to suspend ssl handshake until certificate trust is confirmed. When dart's _RawSecureSocket.secureHandshake() method that initiated ssl's handshake received this return code it knows it has to wait for a future that is completed by another callback([rpEvaluteResponse]) that waits for trust evaluation handler response. rpEvaluateResponse purpose is to listen for response from TrustEvaluateHandler and also invoke user-provided [badCertficateCallback] that can override trust decision for a given connection request, for a given certificate. Once that future is completed and CertificateVerificationCallback knows whether to trust a certificate or not, _secureHandshake retries ssl handshake. Bug: #41519 Change-Id: Ifee18639c78099ec77cad50000444bc6c7b9369b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/165520 Reviewed-by: Siva Annamalai <asiva@google.com> Commit-Queue: Alexander Aprelev <aam@google.com>
Can we close this issue now that the CL has landed ? |
The fix landed without tests. Is it possible to add some to prevent regressions in the future? |
#43674 to track adding a test. |
which version fixed this problem? |
So far this landed as 63852d2073c776d6d849e306b2e36274f2143eb3 in flutter
master channel(
flutter/flutter@63852d2
).
…On Sat, Oct 10, 2020 at 4:41 AM Gentle Kwan ***@***.***> wrote:
which version fixed this problem?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#41519 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC5BUOAIFGQ3MSB4DANNBDSKBB55ANCNFSM4MIZKDZA>
.
|
I've got the same issue, with the above update, I will have to test with what dart version, flutter version and what channel, thank you. |
Add OCSP configrations is not solve this problem!
open the path printed. cd ./internal now you can still using Flutter 1.22.6 but migrate flutter engine to flutter 1.23.0-18.1pre If you can not compile the IOS app after mofications, try modify ./ios/PodFile
and add
then run
now you can flutter run --release |
I would strongly advise against just manually rolling engine like that because you might end up some bizarre issues due to ABI skew between Flutter Framework and Flutter engine. Instead you should always just switch to a newer version of Flutter (e.g. if you want to use engine from 1.23.0-18.1pre you should better use Flutter at 1.23.0-18.1pre as well). |
Using 1.23.0-18.1pre might cause Android GestureDetector not working. we do not have any choice because migrate into Flutter 2.0 is not possible for us at present. But Flutter team might not know this issue is A BIG TROUBLE for Flutter users in China! |
In my application, the SDK used before was 1.9 and everything worked fine. After upgrading to 1.12, android still works well, but ios will get stuck on the splash screen.
Detailed questions:
Code situation
http: 0.12.0+4
My temporary solution
Expected solution
The text was updated successfully, but these errors were encountered: