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

Use QuicheMemSlice constructor with releasor. #31167

Merged
merged 10 commits into from
Dec 12, 2023

Address use-after-move of single_slice_buffer.

44af392
Select commit
Loading
Failed to load commit list.
Merged

Use QuicheMemSlice constructor with releasor. #31167

Address use-after-move of single_slice_buffer.
44af392
Select commit
Loading
Failed to load commit list.
CI (Envoy) / Envoy/Publish and verify succeeded Dec 11, 2023 in 2h 35m 10s

Envoy/Publish and verify (success)

Check has finished

Details

Check run finished (success ✔️)

The check run can be viewed here:

Envoy/Publish and verify (pr/31167/main@44af392)

Check started by

Request (pr/31167/main@44af392)

steveWang @steveWang 44af392 #31167 merge main@00fe753

Use QuicheMemSlice constructor with releasor.

The existing model of forwarding to the quiche::MemSliceImpl constructor is an abstraction violation and makes it hard to swap out the underlying MemSlice platform implementation. As such, we've recently added a new constructor to QuicheMemSlice (and the Impl API) that allows for an arbitrary custom releasor.

This PR uses the new constructor, guarded behind the runtime flag envoy.reloadable_features.quiche_use_mem_slice_releasor_api (disabled by default).

Since we're storing a unique_ptr in the capture list (allowable since C++17), we can't store this in a std::function which requires copyability. As such, we use absl::AnyInvocable.

(I've marked this as medium risk since it interacts with Envoy's memory management model for QUIC streams and so deserves some scrutiny.)

Commit Message: Use QuicheMemSlice constructor with releasor.
Additional Description:
Risk Level: Medium
Testing: This is a refactor, so it should be covered by existing tests (test/common/buffer:buffer_test, test/common/quic:envoy_quic_{client,server}_stream_test)
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Environment

Request variables

Key Value
ref d3536b9a62a6c1b322b7882b1a126f4708f1fe8f
sha 44af392
pr 31167
base-sha 00fe753
actor steveWang @steveWang
message Use QuicheMemSlice constructor with releasor....
started 1702316828.335601
target-branch main
trusted false
Build image

Container image/s (as used in this CI run)

Key Value
default envoyproxy/envoy-build-ubuntu:7467652575122d8d54e767a68f141598bd855383
mobile envoyproxy/envoy-build-ubuntu:mobile-7467652575122d8d54e767a68f141598bd855383
Version

Envoy version (as used in this CI run)

Key Value
major 1
minor 29
patch 0
dev true