Defer sendLocalReply() to prevent reentrant calls.#249

Merged

istio-testing merged 2 commits intoistio:release-1.7from

PiotrSikora:defer_send_local_reply-istio-1.7

Aug 4, 2020

PiotrSikora commented Aug 2, 2020

This was previously fixed in envoyproxy/envoy-wasm#432, but got somehow lost.

Signed-off-by: Piotr Sikora piotrsikora@google.com


          Defer sendLocalReply() to prevent reentrant calls.

6c3e8be

This was previously fixed in envoyproxy/envoy-wasm#432, but got somehow lost.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

istio-testing added the do-not-merge/work-in-progress label

googlebot added the cla: yes label

istio-testing commented Aug 2, 2020

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

istio-testing added the size/XS label

Author

PiotrSikora commented Aug 2, 2020

/test all

PiotrSikora marked this pull request as ready for review

August 3, 2020 01:29

istio-testing removed the do-not-merge/work-in-progress label

PiotrSikora requested a review from jplevyak

August 3, 2020 01:29

Author

PiotrSikora commented Aug 3, 2020

This is a backport of envoyproxy/envoy-wasm#597, please do not merge until it's merged.

/wait

PiotrSikora added the do-not-merge/work-in-progress label

PiotrSikora mentioned this pull request

[release-1.7] Update Envoy-WASM SHA to latest. istio/proxy#2958

Merged

Author

PiotrSikora commented Aug 3, 2020

/test test-tsan_envoy_release-1.7

4 similar comments

Author

PiotrSikora commented Aug 3, 2020

/test test-tsan_envoy_release-1.7

Author

PiotrSikora commented Aug 3, 2020

/test test-tsan_envoy_release-1.7

Author

PiotrSikora commented Aug 3, 2020

/test test-tsan_envoy_release-1.7

Author

PiotrSikora commented Aug 3, 2020

/test test-tsan_envoy_release-1.7

lambdai commented Aug 3, 2020

@PiotrSikora The Context::sendLocalResponse which wraps your changes is supposed to be run in defer. Do you really need a defer during defer?

Author

PiotrSikora commented Aug 3, 2020

@PiotrSikora The Context::sendLocalResponse which wraps your changes is supposed to be run in defer. Do you really need a defer during defer?

I don't believe that's the case. Where did you get that from? cc @jplevyak

Author

PiotrSikora commented Aug 3, 2020

@lambdai also, let's discuss this on the original PR: envoyproxy/envoy-wasm#597


          review: add John's comment.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

istio-testing added size/S and removed size/XS labels

PiotrSikora removed the do-not-merge/work-in-progress label

jplevyak approved these changes

View reviewed changes

istio-testing merged commit bd67b8f into istio:release-1.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes size/S