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

🔥 Do not detaches sockets / terminates client stream #2173

Merged
merged 7 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dio/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ See the [Migration Guide][] for the complete breaking changes list.**

## Unreleased

*None.*
- Remove sockets detach in `IOHttpClientAdapter`.

## 5.4.2+1

Expand Down
3 changes: 0 additions & 3 deletions dio/lib/src/adapters/io_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,6 @@ class IOHttpClientAdapter implements HttpClientAdapter {
.map((e) => RedirectRecord(e.statusCode, e.method, e.location))
.toList(),
statusMessage: responseStream.reasonPhrase,
onClose: () {
responseStream.detachSocket().then((socket) => socket.destroy());
},
);
}

Expand Down
12 changes: 12 additions & 0 deletions dio_test/lib/src/test/cancellation_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -113,5 +113,17 @@ void cancellationTests(
),
);
});

// Regression: https://github.com/cfug/dio/issues/2170
test(
'not closing sockets with requests that have same hosts',
() async {
final token = CancelToken();
await expectLater(dio.get('/get', cancelToken: token), completes);
expectLater(dio.get('/drip?duration=3'), completes);
Future.delayed(const Duration(seconds: 1), token.cancel);
},
testOn: '!browser',
);
});
}
22 changes: 16 additions & 6 deletions dio_test/lib/src/test/download_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,12 @@ void downloadTests(
]);
await expectLater(
dio.downloadUri(
Uri.parse('/drip?delay=0&duration=6&numbytes=6').replace(
queryParameters: {'count': '3', 'gap': '2'},
Uri.parse('/drip').replace(
queryParameters: {
'delay': '0',
'duration': '4',
'numbytes': '2',
},
),
p.join(tmp.path, 'download_timeout.md'),
options: Options(receiveTimeout: Duration(seconds: 1)),
Expand All @@ -264,10 +268,16 @@ void downloadTests(
);

// Throws nothing if it constantly gets response bytes.
await dio.download(
'https://github.com/cfug/flutter.cn/archive/refs/heads/main.zip',
p.join(tmp.path, 'main.zip'),
options: Options(receiveTimeout: Duration(seconds: 1)),
await dio.downloadUri(
Uri.parse('/drip').replace(
queryParameters: {
'delay': '0',
'duration': '5',
'numbytes': '5',
},
),
p.join(tmp.path, 'download_timeout.md'),
options: Options(receiveTimeout: Duration(seconds: 2)),
);
},
// The download of the main.zip file can be slow,
Expand Down
2 changes: 1 addition & 1 deletion plugins/http2_adapter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Unreleased

*None.*
- Remove client stream termination in `Http2Adapter`.

## 2.5.1

Expand Down
15 changes: 7 additions & 8 deletions plugins/http2_adapter/lib/src/http2_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,14 @@ class Http2Adapter implements HttpClientAdapter {
// Creates a new outgoing stream.
final stream = transport.makeRequest(headers);

cancelFuture?.whenComplete(() {
Future(() {
stream.terminate();
final hasRequestData = requestStream != null;
if (hasRequestData) {
cancelFuture?.whenComplete(() {
stream.outgoingMessages.close();
});
});
}

List<Uint8List>? list;
final hasRequestData = requestStream != null;
if (!excludeMethods.contains(options.method) && hasRequestData) {
list = await requestStream.toList();
requestStream = Stream.fromIterable(list);
Expand All @@ -145,7 +145,7 @@ class Http2Adapter implements HttpClientAdapter {
requestStreamFuture = requestStreamFuture.timeout(
sendTimeout,
onTimeout: () {
stream.terminate();
stream.outgoingMessages.close();
throw DioException.sendTimeout(
timeout: sendTimeout,
requestOptions: options,
Expand Down Expand Up @@ -195,7 +195,6 @@ class Http2Adapter implements HttpClientAdapter {
);
} else {
responseSubscription.cancel().whenComplete(() {
stream.terminate();
responseSink.close();
});
}
Expand Down Expand Up @@ -277,7 +276,7 @@ class Http2Adapter implements HttpClientAdapter {
onClose: () {
responseSubscription.cancel();
responseSink.close();
stream.terminate();
stream.outgoingMessages.close();
},
);
}
Expand Down
Loading