Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crash in rule_tree::pop_from_free_list #14213

Closed
bholley opened this issue Nov 14, 2016 · 3 comments
Closed

crash in rule_tree::pop_from_free_list #14213

bholley opened this issue Nov 14, 2016 · 3 comments
Labels

Comments

@bholley
Copy link
Contributor

@bholley bholley commented Nov 14, 2016

Loading http://spectrum.ieee.org/video/geek-life/tools-toys/this-water-jet-cutter-can-slice-through-anything-steel-glass-or-steak in Servo, I hit the following:

https://pastebin.mozilla.org/8928296

The stack from lldb is a bit more illuminating: https://pastebin.mozilla.org/8928301

Basically, we're finalizing the Window on the script thread, and then we end up in alloc::arc::{{impl}}::drop<std::sync::mutex::Mutexlayout::query::LayoutThreadData>

That seems to suggest that we're dropping an Arc to layout data on the script thread, which seems problematic.

LayoutRPCImpl looks questionable:

pub struct LayoutRPCImpl(pub Arc<Mutex<LayoutThreadData>>);

And we send it over to the layout thread here:

response_chan.send(box LayoutRPCImpl(self.rw_data.clone()) as

We've got an Arc<Mutex>, so from the Type system's perspective this is all kosher. Do we need to mark StrongRuleNode as non-Send? Or should we remove the assertion?

CC @emilio @jdm

@emilio
Copy link
Member

@emilio emilio commented Nov 14, 2016

Ok, I was worried for a second this could be dropping node's data inside script.

We're finalizing the window after layout, which I think it's... not quite the expected, and probably should be investigated a bit more.

Probably the assertion should be removed, but I think it's not quite the expected thing in any case.

@jdm jdm added I-crash I-panic and removed I-crash labels Nov 14, 2016
@jdm
Copy link
Member

@jdm jdm commented Nov 14, 2016

I'll defer to people like @emilio and @pcwalton here.

bors-servo added a commit that referenced this issue Nov 18, 2016
style: Don't assert when the final rule tree GC happens in script.

This should silence the assertions in #14213.

r? @Ms2ger

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14278)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Nov 18, 2016
style: Don't assert when the final rule tree GC happens in script.

This should silence the assertions in #14213.

r? @Ms2ger

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14278)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Nov 20, 2016
style: Don't assert when the final rule tree GC happens in script.

This should silence the assertions in #14213.

r? @Ms2ger

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14278)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Nov 20, 2016
style: Don't assert when the final rule tree GC happens in script.

This should silence the assertions in #14213.

r? @Ms2ger

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14278)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 4, 2017
… happens in script (from emilio:layout-data-in-script); r=Ms2ger

This should silence the assertions in servo/servo#14213.

r? @Ms2ger

Source-Repo: https://github.com/servo/servo
Source-Revision: 976989fc8d6a61391b35639943c63e6aaa27068b
@nox
Copy link
Member

@nox nox commented Oct 7, 2017

Original page doesn't panic anymore, and #14278 is supposed to have fixed the problem. Closing this.

@nox nox closed this Oct 7, 2017
@pyfisch pyfisch mentioned this issue Jan 4, 2018
4 of 13 tasks complete
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
… happens in script (from emilio:layout-data-in-script); r=Ms2ger

This should silence the assertions in servo/servo#14213.

r? Ms2ger

Source-Repo: https://github.com/servo/servo
Source-Revision: 976989fc8d6a61391b35639943c63e6aaa27068b

UltraBlame original commit: 5def761b9b1762fa9b316f5c4c92dbb576dd11c3
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
… happens in script (from emilio:layout-data-in-script); r=Ms2ger

This should silence the assertions in servo/servo#14213.

r? Ms2ger

Source-Repo: https://github.com/servo/servo
Source-Revision: 976989fc8d6a61391b35639943c63e6aaa27068b

UltraBlame original commit: 5def761b9b1762fa9b316f5c4c92dbb576dd11c3
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
… happens in script (from emilio:layout-data-in-script); r=Ms2ger

This should silence the assertions in servo/servo#14213.

r? Ms2ger

Source-Repo: https://github.com/servo/servo
Source-Revision: 976989fc8d6a61391b35639943c63e6aaa27068b

UltraBlame original commit: 5def761b9b1762fa9b316f5c4c92dbb576dd11c3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.