Added ability to randomly kill pipelines to the constellation. #10179

asajeffrey · 2016-03-24T20:56:34Z

Added flags:

--random-pipeline-closure-probability: probability of each event triggering a forced exit of a randomly chosen pipeline.
--random-pipeline-closure-seed: seed to use for the RNG (to remove a source of intermittent failure).

These are designed for testing the hardness of the constellation.

This change is

asajeffrey · 2016-03-24T21:16:08Z

Things that we need to make sure about this PR:

If --random-pipeline-closure-probability isn't set, the PR changes nothing.
There's no security implications, even if an attacker manages to get servo into zombie apocalypse mode.
I think I managed both of these, but you never know...

asajeffrey · 2016-03-24T21:22:25Z

Argh, forgot to run test-tidy.

asajeffrey · 2016-03-25T14:03:01Z

There is a constellation hardening metabug: issue #10124.

emilio · 2016-03-25T15:21:15Z

Looks good to me!

I left a comment about the close_random_pipeline function, but I could tolerate landing it as-is if you think being clearer there is unnecessary, so feel free to r=me if that's the case :)

-S-awaiting-review +S-awaiting-answer

Reviewed 4 of 6 files at r1, 1 of 2 files at r2.
Review status: 5 of 6 files reviewed at latest revision, 1 unresolved discussion.

components/compositing/constellation.rs, line 370 [r2] (raw file):
I'd include in this comment something like "if --random-pipeline-closure-probability was specified at startup" or similar, just to make it clear when scanning the constellation code (in order to not having to look ~1000 lines below).

Renaming close_random_pipeline to maybe_close_random_pipeline, or even changing the signature to close_random_pipeline(&mut self, rng: &mut StdRng, probability: f32), so it would be obvious from here that this will do nothing unless random_pipeline_closure is set would help.

These are nits though, and I won't blame you for landing this as-is :P

Comments from the review on Reviewable.io

asajeffrey · 2016-03-25T15:47:02Z

Review status: 5 of 6 files reviewed at latest revision, 1 unresolved discussion.

components/compositing/constellation.rs, line 370 [r2] (raw file):
Added some comments, and renamed close_random_pipeline to maybe_close_random_pipeline. Thanks for the review!

Comments from the review on Reviewable.io

mbrubeck · 2016-03-25T16:07:44Z

components/compositing/constellation.rs

@@ -347,6 +352,11 @@ impl<LTF: LayoutThreadFactory, STF: ScriptThreadFactory> Constellation<LTF, STF>
                child_processes: Vec::new(),
                document_states: HashMap::new(),
                webrender_api_sender: state.webrender_api_sender,
+                random_pipeline_closure: opts::get().random_pipeline_closure_probability.map(|prob| {
+                    let seed = opts::get().random_pipeline_closure_seed.unwrap_or_else(random);


When it a random seed, perhaps it should log the seed.

Done. Should we log using debug! or something else?

debug! is compiled out in release builds of Servo, so use info! if you want to use this feature in release.

emilio · 2016-03-25T18:54:00Z

Looks good to me, r=me with that typo fixed and the commits squashed :)

-S-awaiting-answer -S-awaiting-answer +S-needs-code-changes +S-needs-squash

Reviewed 1 of 2 files at r2, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

components/compositing/constellation.rs, line 370 [r2] (raw file):
nit: s/pipelne/pipeline

Comments from the review on Reviewable.io

asajeffrey · 2016-03-25T22:28:24Z

Squashed, typo fixed.

Review status: 5 of 6 files reviewed at latest revision, 1 unresolved discussion.

Comments from the review on Reviewable.io

asajeffrey · 2016-03-25T22:33:47Z

@bors-servo: r=emilio

bors-servo · 2016-03-25T22:33:48Z

📌 Commit 1ec222c has been approved by emilio

bors-servo · 2016-03-26T00:07:07Z

⌛ Testing commit 1ec222c with merge fde06cc...

Added ability to randomly kill pipelines to the constellation. Added flags: * `--random-pipeline-closure-probability`: probability of each event triggering a forced exit of a randomly chosen pipeline. * `--random-pipeline-closure-seed`: seed to use for the RNG (to remove a source of intermittent failure). These are designed for testing the hardness of the constellation.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10179)

bors-servo · 2016-03-26T00:18:24Z

💔 Test failed - gonk

emilio · 2016-03-26T00:33:38Z

Need to update the other lockfiles.

asajeffrey · 2016-03-26T02:29:06Z

Grr, I'll do this tomorrow.

asajeffrey · 2016-03-26T14:12:57Z

OK, fixed Cargo.lock files, Let's see if this works...

@bors-servo try

bors-servo · 2016-03-26T14:12:59Z

⌛ Trying commit 32c72f0 with merge 8a13b7b...

Added ability to randomly kill pipelines to the constellation. Added flags: * `--random-pipeline-closure-probability`: probability of each event triggering a forced exit of a randomly chosen pipeline. * `--random-pipeline-closure-seed`: seed to use for the RNG (to remove a source of intermittent failure). These are designed for testing the hardness of the constellation.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10179)

bors-servo · 2016-03-26T15:41:11Z

☀️ Test successful - android, arm32, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor

asajeffrey · 2016-03-26T15:45:58Z

@bors-servo: r=emilio

bors-servo · 2016-03-26T15:45:59Z

📌 Commit 32c72f0 has been approved by emilio

bors-servo · 2016-03-26T15:46:00Z

⌛ Testing commit 32c72f0 with merge dc0e541...

Added ability to randomly kill pipelines to the constellation. Added flags: * `--random-pipeline-closure-probability`: probability of each event triggering a forced exit of a randomly chosen pipeline. * `--random-pipeline-closure-seed`: seed to use for the RNG (to remove a source of intermittent failure). These are designed for testing the hardness of the constellation.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10179)

bors-servo · 2016-03-26T16:33:06Z

💔 Test failed - linux-rel

asajeffrey · 2016-03-26T17:05:06Z

@bors-servo: retry

Unexpected timeout in /websockets/constructor/009.html #9818

bors-servo · 2016-03-26T17:05:08Z

⚡ Previous build results for android, arm32, gonk, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor are reusable. Rebuilding only linux-rel...

bors-servo · 2016-03-26T17:52:23Z

☀️ Test successful - android, arm32, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor

highfive assigned emilio Mar 24, 2016

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 24, 2016

asajeffrey force-pushed the chaotic-good branch from 9cfa85f to e661fa3 Compare March 24, 2016 21:21

asajeffrey mentioned this pull request Mar 25, 2016

Constellation hardening metabug #10124

Closed

emilio added S-awaiting-answer Someone asked a question that requires an answer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 25, 2016

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 25, 2016

mbrubeck reviewed Mar 25, 2016
View reviewed changes

emilio added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-needs-squash Some (or all) of the commits in the PR should be combined. and removed S-awaiting-answer Someone asked a question that requires an answer. labels Mar 25, 2016

asajeffrey force-pushed the chaotic-good branch from 6e0863f to 1ec222c Compare March 25, 2016 22:27

highfive removed the S-needs-code-changes Changes have not yet been made that were requested by a reviewer. label Mar 25, 2016

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-squash Some (or all) of the commits in the PR should be combined. labels Mar 25, 2016

asajeffrey mentioned this pull request Mar 25, 2016

Add reproducible sources of panic to tests #10144

Open

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 26, 2016

highfive added the S-tests-failed The changes caused existing tests to fail. label Mar 26, 2016

asajeffrey force-pushed the chaotic-good branch from 1ec222c to 8f9b073 Compare March 26, 2016 14:00

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Mar 26, 2016

asajeffrey force-pushed the chaotic-good branch from 8f9b073 to 2c04724 Compare March 26, 2016 14:03

Added ability to randomly kill pipelines to the constellation.

32c72f0

asajeffrey force-pushed the chaotic-good branch from 2c04724 to 32c72f0 Compare March 26, 2016 14:11

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 26, 2016

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 26, 2016

bors-servo merged commit 32c72f0 into servo:master Mar 26, 2016

asajeffrey deleted the chaotic-good branch March 26, 2016 18:27

asajeffrey mentioned this pull request Mar 28, 2016

Removed panicking when frame or pipeline lookup fails. #10082

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added ability to randomly kill pipelines to the constellation. #10179

Added ability to randomly kill pipelines to the constellation. #10179

asajeffrey commented Mar 24, 2016

asajeffrey commented Mar 24, 2016

asajeffrey commented Mar 24, 2016

asajeffrey commented Mar 25, 2016

emilio commented Mar 25, 2016

asajeffrey commented Mar 25, 2016

mbrubeck Mar 25, 2016

asajeffrey Mar 25, 2016

mbrubeck Mar 25, 2016

asajeffrey Mar 25, 2016

emilio commented Mar 25, 2016

asajeffrey commented Mar 25, 2016

asajeffrey commented Mar 25, 2016

bors-servo commented Mar 25, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

emilio commented Mar 26, 2016

asajeffrey commented Mar 26, 2016

asajeffrey commented Mar 26, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

asajeffrey commented Mar 26, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

asajeffrey commented Mar 26, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

Added ability to randomly kill pipelines to the constellation. #10179

Added ability to randomly kill pipelines to the constellation. #10179

Conversation

asajeffrey commented Mar 24, 2016

asajeffrey commented Mar 24, 2016

asajeffrey commented Mar 24, 2016

asajeffrey commented Mar 25, 2016

emilio commented Mar 25, 2016

asajeffrey commented Mar 25, 2016

mbrubeck Mar 25, 2016

Choose a reason for hiding this comment

asajeffrey Mar 25, 2016

Choose a reason for hiding this comment

mbrubeck Mar 25, 2016

Choose a reason for hiding this comment

asajeffrey Mar 25, 2016

Choose a reason for hiding this comment

emilio commented Mar 25, 2016

asajeffrey commented Mar 25, 2016

asajeffrey commented Mar 25, 2016

bors-servo commented Mar 25, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

emilio commented Mar 26, 2016

asajeffrey commented Mar 26, 2016

asajeffrey commented Mar 26, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

asajeffrey commented Mar 26, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016

asajeffrey commented Mar 26, 2016

bors-servo commented Mar 26, 2016

bors-servo commented Mar 26, 2016