Skip to content

fixes race condition in Run by decoupling singal handling from the impl#458

Merged
mathetake merged 6 commits intomasterfrom
racecondition
Mar 19, 2025
Merged

fixes race condition in Run by decoupling singal handling from the impl#458
mathetake merged 6 commits intomasterfrom
racecondition

Conversation

@mathetake
Copy link
Copy Markdown
Contributor

Redo of #456. Basically, the original impl had a race condition and cannot wait for the Envoy processes to actually exit plus there was a race condition when accessing exec.Process objects. This fixes it altogether by decoupling internal/envoy/run implementation from the actual signal handling and making it focus on the simple context cancelation.

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Copy link
Copy Markdown
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, this wasn't originally designed to be called as a library, so moving the signal handling to main makes sense.

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
Copy link
Copy Markdown
Contributor Author

The tests running both on EG (envoyproxy/gateway#5527) and AIGW (envoyproxy/ai-gateway#458) using this branch. good to go

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake mathetake merged commit ca8702e into master Mar 19, 2025
7 checks passed
@mathetake mathetake deleted the racecondition branch March 19, 2025 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants