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

Adds borrow_for_script_deallocation and unsafe_mut_js_info method to avo... #4777

Closed
wants to merge 1 commit into from

Conversation

@dmarcos
Copy link
Contributor

dmarcos commented Jan 30, 2015

...id 'DOMRefCell already mutably borrowed' messages. This is just a temporary fix until the Rust standard library allows borrowing already-borrowed RefCell values during unwinding.

It also removes LiveDOMReferences destructor that it's a no-op but it contains an assert that was being violated causing an endless cycle of destructor calls ending up in a stack overflow.

@highfive
Copy link

highfive commented Jan 30, 2015

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @kmcallister (or someone else) soon.

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jan 30, 2015

Critic review: https://critic.hoppipolla.co.uk/r/3874

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@highfive
Copy link

highfive commented Jan 30, 2015

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@jdm
Copy link
Member

jdm commented Jan 30, 2015

Thanks for sticking with this @dmarcos! I've left two small comments on Critic that should be addressed in an additional commit :)

@dmarcos
Copy link
Contributor Author

dmarcos commented Jan 30, 2015

@jdm. Thanks! I addressed the review comments. Are we done? :)

@jdm
Copy link
Member

jdm commented Jan 30, 2015

Squash and I'll merge :)

@dmarcos dmarcos force-pushed the dmarcos:issue4692 branch from f24af24 to 8439fc0 Jan 30, 2015
@dmarcos
Copy link
Contributor Author

dmarcos commented Jan 30, 2015

Done! :)

@dmarcos dmarcos force-pushed the dmarcos:issue4692 branch from 8439fc0 to bde8c4a Jan 30, 2015
@dmarcos
Copy link
Contributor Author

dmarcos commented Jan 30, 2015

@jdm Looking now for a second bug to work on. I've seen some work done on the canvas API. Is there any easy stuff on that front that I can tackle? Thanks for your help

@jdm jdm added S-awaiting-merge and removed S-needs-squash labels Jan 30, 2015
bors-servo pushed a commit that referenced this pull request Jan 30, 2015
...id 'DOMRefCell already mutably borrowed' messages. This is just a temporary fix until the Rust standard library allows borrowing already-borrowed RefCell values during unwinding.

It also removes LiveDOMReferences destructor that it's a no-op but it contains an assert that was being violated causing an endless cycle of destructor calls ending up in a stack overflow.
…avoid 'DOMRefCell already mutably borrowed' messages. This is just a temporary fix until the Rust standard library allows borrowing already-borrowed RefCell values during unwinding.

It also removes LiveDOMReferences destructor that it's a no-op but it contains an assert that was being violated causing an endless cycle of destructor calls ending up in a stack overflow.
@dmarcos dmarcos force-pushed the dmarcos:issue4692 branch from bde8c4a to 7b9c902 Jan 30, 2015
@jdm

This comment has been minimized.

Copy link

jdm commented on 7b9c902 Jan 30, 2015

r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 7b9c902 Jan 30, 2015

saw approval from jdm
at dmarcos@7b9c902

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 30, 2015

merging dmarcos/servo/issue4692 = 7b9c902 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 30, 2015

dmarcos/servo/issue4692 = 7b9c902 merged ok, testing candidate = a7e2993

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 30, 2015

fast-forwarding master to auto = a7e2993

bors-servo pushed a commit that referenced this pull request Jan 30, 2015
...id 'DOMRefCell already mutably borrowed' messages. This is just a temporary fix until the Rust standard library allows borrowing already-borrowed RefCell values during unwinding.

It also removes LiveDOMReferences destructor that it's a no-op but it contains an assert that was being violated causing an endless cycle of destructor calls ending up in a stack overflow.
@bors-servo bors-servo closed this Jan 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.