compositing: Split Servo up into multiple sandboxed processes. #6884

pcwalton · 2015-08-01T06:06:14Z

Multiprocess mode is enabled with the -M switch, and sandboxing is
enabled with the -S switch.

highfive · 2015-08-01T06:06:21Z

Warning

These commits modify unsafe code. Please review it carefully!

pcwalton · 2015-08-01T06:06:56Z

Note that the processes don't shut down properly yet, for multiple reasons: shutdown channels aren't wired up yet and we don't wait on children, so they become zombies. But I didn't want to make this patch bigger.

jdm · 2015-08-01T15:01:28Z

Review status: 0 of 30 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

^{components/devtools/actors/timeline.rs, line 205 [r1] (raw file):}
This change is no longer necessary.

Comments from the review on Reviewable.io

metajack · 2015-08-04T06:26:12Z

Reviewed 21 of 30 files at r1.
Review status: 21 of 30 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

^{components/util/task.rs, line 49 [r1] (raw file):}
Is there really no way to abstract this over the Sender/IpcSender? This seems like a terrible amount of code duplication.

Comments from the review on Reviewable.io

metajack · 2015-08-04T20:31:40Z

-S-awaiting-review +S-needs-code-changes

I think ipc-channel needs more documentation. Some of its uses are pretty hard to grok from the code/comments written.

This looks ok except for the comments I left, which were mostly about docs.

Reviewed 9 of 30 files at r1.
Review status: all files reviewed at latest revision, 7 unresolved discussions, all commit checks successful.

^{components/layout/layout_task.rs, line 734 [r1] (raw file):}
This is a quite convoluted way to have a one shot receiver. Please file a followup bug to make a better API for this in ipc-channel. For example, ROUTER.add_oneshot_route(...).

For this patch, please document that a oneshot route is what you're doing here, because it's not clear at all unless you really squint.

^{components/script/dom/htmllinkelement.rs, line 263 [r1] (raw file):}
Why do we need to clone here? Can't we call self.script_chan.send(...) directly?

^{components/script/script_task.rs, line 532 [r1] (raw file):}
Why? We already have a control port. Won't we get these messages without proxying them?

^{components/servo/lib.rs, line 222 [r1] (raw file):}
This comment is very confusingly written. If "this" is the immediately following block, then the comment is contradictory.

Please find another way to express whatever you are trying to communicate here :)

^{components/servo/lib.rs, line 235 [r1] (raw file):}
Does anything ever force kill this process? Or does this process always hang?

Should we really land this patch with this?

Comments from the review on Reviewable.io

bors-servo · 2015-08-05T15:21:48Z

☔ The latest upstream changes (presumably #6416) made this pull request unmergeable. Please resolve the merge conflicts.

metajack · 2015-09-01T15:51:26Z

@pcwalton Any updates here? I reviewed this a month ago.

pcwalton · 2015-09-30T00:38:32Z

Rebased up to now. Multiprocess is working but sandboxing is not. This may be Mac OS X Yosemite-specific.

pcwalton · 2015-09-30T01:02:26Z

Review status: 0 of 34 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

^{components/script/dom/htmllinkelement.rs, line 0 [r1] (raw file):}
Can't do that because that will borrow self. I'll add a comment to make this clearer.

Comments from the review on Reviewable.io

pcwalton · 2015-09-30T01:05:20Z

About half of the review comments addressed so far.

bors-servo · 2015-09-30T01:59:08Z

☔ The latest upstream changes (presumably #7423) made this pull request unmergeable. Please resolve the merge conflicts.

pcwalton · 2015-09-30T22:30:26Z

Review status: 0 of 33 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

^{components/script/script_task.rs, line 532 [r1] (raw file):}
It's because we select over lots of ports, and we can't select over in-process and IPC ports simultaneously. So we have to proxy IPC ports to in-process ports so that all the ports we're selecting over are of the same type (in this case, in-process ports).

Comments from the review on Reviewable.io

pcwalton · 2015-09-30T22:33:27Z

Review status: 0 of 33 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

^{components/servo/lib.rs, line 222 [r1] (raw file):}
You're right, this comment makes no sense. I don't even remember what I meant here. Deleting it.

Comments from the review on Reviewable.io

pcwalton · 2015-09-30T23:19:17Z

Comments addressed. Content processes should shut down now, though it's hard to test this since we don't actually evict pipelines from the bfcache!

r? @metajack

bors-servo · 2015-10-15T14:40:12Z

💔 Test failed - gonk

bors-servo · 2015-10-20T22:47:35Z

☔ The latest upstream changes (presumably #7950) made this pull request unmergeable. Please resolve the merge conflicts.

Closes servo#8235.

Multiprocess mode is enabled with the `-M` switch, and sandboxing is enabled with the `-S` switch.

pcwalton · 2015-10-28T03:08:12Z

Rebased. Now depends on #8237

bors-servo · 2015-10-28T17:19:03Z

☔ The latest upstream changes (presumably #8241) made this pull request unmergeable. Please resolve the merge conflicts.

jdm · 2015-11-13T04:09:45Z

#8492 merged! Go go go go go go go!

compositing: Split Servo up into multiple sandboxed processes. Multiprocess mode is enabled with the `-M` switch, and sandboxing is enabled with the `-S` switch. Rebase of #6884.

jdm · 2015-11-19T07:35:54Z

Redone in #8599.

compositing: Split Servo up into multiple sandboxed processes. Multiprocess mode is enabled with the `-M` switch, and sandboxing is enabled with the `-S` switch. Rebase of #6884.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8599)

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 1, 2015

metajack self-assigned this Aug 3, 2015

metajack added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 4, 2015

pcwalton force-pushed the e10s-redux branch from 5319245 to ab992d2 Compare September 30, 2015 00:37

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Sep 30, 2015

pcwalton force-pushed the e10s-redux branch from ab992d2 to e5c9657 Compare September 30, 2015 00:43

pcwalton mentioned this pull request Sep 30, 2015

Allow one-shot routes servo/ipc-channel#9

Open

pcwalton force-pushed the e10s-redux branch from e5c9657 to 536f114 Compare September 30, 2015 01:04

highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Sep 30, 2015

pcwalton force-pushed the e10s-redux branch from 536f114 to 4b8b04c Compare September 30, 2015 22:28

pcwalton force-pushed the e10s-redux branch 2 times, most recently from 811dfd8 to 6baae53 Compare September 30, 2015 23:18

pcwalton force-pushed the e10s-redux branch from 6baae53 to 8459005 Compare September 30, 2015 23:23

highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 15, 2015

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 Oct 15, 2015

highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 20, 2015

pcwalton added 2 commits October 27, 2015 20:01

script: Make timer events e10s-safe.

ac618ec

Closes servo#8235.

compositing: Split Servo up into multiple sandboxed processes.

d4f47da

Multiprocess mode is enabled with the `-M` switch, and sandboxing is enabled with the `-S` switch.

pcwalton force-pushed the e10s-redux branch from c67884c to d4f47da Compare October 28, 2015 03:07

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 Oct 28, 2015

pcwalton added S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Oct 28, 2015

highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 28, 2015

jdm mentioned this pull request Oct 28, 2015

Split Servo into multiple processes, and introduce simple sandboxing #4735

Closed

jdm removed the S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. label Nov 13, 2015

jdm mentioned this pull request Nov 19, 2015

compositing: Split Servo up into multiple sandboxed processes. #8599

Merged

jdm closed this Nov 19, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compositing: Split Servo up into multiple sandboxed processes. #6884

compositing: Split Servo up into multiple sandboxed processes. #6884

pcwalton commented Aug 1, 2015

highfive commented Aug 1, 2015

pcwalton commented Aug 1, 2015

jdm commented Aug 1, 2015

metajack commented Aug 4, 2015

metajack commented Aug 4, 2015

bors-servo commented Aug 5, 2015

metajack commented Sep 1, 2015

pcwalton commented Sep 30, 2015

pcwalton commented Sep 30, 2015

pcwalton commented Sep 30, 2015

bors-servo commented Sep 30, 2015

pcwalton commented Sep 30, 2015

pcwalton commented Sep 30, 2015

pcwalton commented Sep 30, 2015

bors-servo commented Oct 15, 2015

bors-servo commented Oct 20, 2015

pcwalton commented Oct 28, 2015

bors-servo commented Oct 28, 2015

jdm commented Nov 13, 2015

jdm commented Nov 19, 2015

compositing: Split Servo up into multiple sandboxed processes. #6884

compositing: Split Servo up into multiple sandboxed processes. #6884

Conversation

pcwalton commented Aug 1, 2015

highfive commented Aug 1, 2015

pcwalton commented Aug 1, 2015

jdm commented Aug 1, 2015

metajack commented Aug 4, 2015

metajack commented Aug 4, 2015

bors-servo commented Aug 5, 2015

metajack commented Sep 1, 2015

pcwalton commented Sep 30, 2015

pcwalton commented Sep 30, 2015

pcwalton commented Sep 30, 2015

bors-servo commented Sep 30, 2015

pcwalton commented Sep 30, 2015

pcwalton commented Sep 30, 2015

pcwalton commented Sep 30, 2015

bors-servo commented Oct 15, 2015

bors-servo commented Oct 20, 2015

pcwalton commented Oct 28, 2015

bors-servo commented Oct 28, 2015

jdm commented Nov 13, 2015

jdm commented Nov 19, 2015