Fix browser not stop during aborts (e.g. SIGTERM) #1420
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ankur22
commented
Sep 10, 2024
ankur22
commented
Sep 10, 2024
ankur22
force-pushed
the
fix/browser-not-stop-sigterm
branch
from
a94d2da
to
e345ffe
Compare
ankur22
force-pushed
the
fix/browser-not-stop-sigterm
branch
from
e345ffe
to
02fd78d
Compare
3 tasks
ankur22
force-pushed
the
fix/browser-not-stop-sigterm
branch
2 times, most recently
from
82476ee
to
18c626b
Compare
This helps us by clarifying that this is a background context, and not controlled by k6 (i.e. the VU context). The background context is to only be used to control the lifecycle of chromium/connection. In later commits vu context will be reintroduced to control the iteration lifecycle so that k6 can abort the iteration and the browser module doesn't block that from happening.
The background context will be used for the chromium subprocess, connection and things that need to be controlled by the iterStart and iterEnd (k6 event system) events. The k6Ctx is the context that is controlled by the vu context. This will allow the iteration to abort due to a SIGTERM while also allowing chromium to exit with endIter/exit events. At the moment this only introduces the k6Ctx, but it is still the background context.
vuCtx is a clearer name which represents the VU context.
Now that Browser is setup to work with the different contexts, let's actually set them up correctly.
This rename makes it clearer which context will be cancelled if the function is called.
The background context isn't needed here. We're removing it to make things clearer later on when we get to creating the Browser type, which uses the cu context.
It didn't feel right having a goroutine where there was no way of shutting it down. Now when the browser.Close() method is called it will shut the init goroutine down too.
ankur22
force-pushed
the
fix/browser-not-stop-sigterm
branch
from
d804f1f
to
dbac2f9
Compare
This is to ensure that we remember why we're using context.Background for launching or connecting to chromium.
Encapsulate the retrieval of the browser count from the registry.
Encapsulate the retrieval of the iteration trace count from the registry.
inancgumus
approved these changes
Sep 12, 2024
To be more idiomatic go
inancgumus
approved these changes
Sep 12, 2024
This was referenced Sep 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This change fixes an issue whereby the browser module could block a test abort if the browser module was in the middle of waiting on something e.g. working with
waitFor*apis.The fix is to use the VU context for the iteration, but keep to the background context for running the chromium subprocess and connections. Now when the test is aborted, the vu context is closed by k6, which in turn will cancel all running browser API calls, and finally allow k6 to shutdown the test.
Why?
It was blocking the test from aborting. The reason was due to the use of the background context for the iteration as well as for running the browser subprocess and connection. k6 signals a test abort by cancelling the VU context, but since the browser module wasn't relying on that, the browser module would first complete the exiting API call/iteration before realising that the test was aborted.
We need to work with both the VU context to control the iteration lifecycle, and the background context so that we allow the k6 event system to control the chromium subprocess/connection lifecycle. They need to be separate to prevent race conditions when working with the chromium subprocess/connection.
Checklist
Related PR(s)/Issue(s)
Updates: #1410