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

Use webvr future_frame_data to avoid blocking the WebGL thread #22938

Merged
merged 1 commit into from Feb 27, 2019

Conversation

Projects
None yet
6 participants
@asajeffrey
Copy link
Member

asajeffrey commented Feb 25, 2019

This PR fixes a potential deadlock caused by the WebGL thread being blocked on a VR device. Rather than blocking on the VR device, it forwards a future to the script thread, and then then script thread blocks.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because it's fixing a potential deadlock

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

highfive commented Feb 25, 2019

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/vrdisplay.rs
@highfive

This comment has been minimized.

Copy link

highfive commented Feb 25, 2019

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify script code, but no tests are modified. Please consider adding a test!
@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Feb 25, 2019

Shouldn't land until servo/rust-webvr#62 does.

@paulrouget

This comment has been minimized.

Copy link
Contributor

paulrouget commented Feb 26, 2019

r=me once the webvr part lands.

@asajeffrey asajeffrey force-pushed the asajeffrey:webvr-future-frame-data branch from a4b0e97 to c908701 Feb 26, 2019

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Feb 26, 2019

@bors-servo r=paulrouget

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 26, 2019

📌 Commit c908701 has been approved by paulrouget

@highfive highfive assigned paulrouget and unassigned jdm Feb 26, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 26, 2019

⌛️ Testing commit c908701 with merge aa46e0c...

bors-servo added a commit that referenced this pull request Feb 26, 2019

Auto merge of #22938 - asajeffrey:webvr-future-frame-data, r=paulrouget
Use webvr future_frame_data to avoid blocking the WebGL thread

<!-- Please describe your changes on the following line: -->
This PR fixes a potential deadlock caused by the WebGL thread being blocked on a VR device. Rather than blocking on the VR device, it forwards a future to the script thread, and then then script thread blocks.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because it's fixing a potential deadlock

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22938)
<!-- Reviewable:end -->

@asajeffrey asajeffrey referenced this pull request Feb 26, 2019

Merged

Pass the GL context to the VRDisplay when rendering #22939

3 of 3 tasks complete
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 26, 2019

💔 Test failed - status-taskcluster

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Feb 26, 2019

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 26, 2019

💣 Failed to start rebuilding: Unknown error

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 26, 2019

⌛️ Testing commit c908701 with merge dee97af...

bors-servo added a commit that referenced this pull request Feb 26, 2019

Auto merge of #22938 - asajeffrey:webvr-future-frame-data, r=paulrouget
Use webvr future_frame_data to avoid blocking the WebGL thread

<!-- Please describe your changes on the following line: -->
This PR fixes a potential deadlock caused by the WebGL thread being blocked on a VR device. Rather than blocking on the VR device, it forwards a future to the script thread, and then then script thread blocks.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because it's fixing a potential deadlock

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22938)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 26, 2019

💔 Test failed - status-taskcluster

@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Feb 27, 2019

  ▶ CRASH [expected TIMEOUT] /html/browsers/windows/nested-browsing-contexts/name-attribute.window.html
  │ 
  │ VMware, Inc.
  │ softpipe
  │ 3.3 (Core Profile) Mesa 18.3.0-devel
  │ Layout thread disconnected.: "SendError(..)" (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }, at src/libcore/result.rs:997)
  │ stack backtrace:
  │    0:        0x1077244be - __ZN9backtrace9backtrace5trace17h9194fd11aa5e0269E
  │    1:        0x10772344c - __ZN72_$LT$backtrace..capture..Backtrace$u20$as$u20$core..default..Default$GT$7default17h4bf15fdf689dde05E
  │    2:        0x105508ab9 - __ZN5servo4main28_$u7b$$u7b$closure$u7d$$u7d$17he767bddbf614fd40E
  │    3:        0x1081a51b0 - __ZN3std9panicking20rust_panic_with_hook17hdb6ee47c165b46dcE
  │    4:        0x1081a4bfc - __ZN3std9panicking18continue_panic_fmt17h17fd7d2759b7149cE
  │    5:        0x1081a4ae8 - _rust_begin_unwind
  │    6:        0x1081c27b1 - __ZN4core9panicking9panic_fmt17h254770d6c8ab828aE
  │    7:        0x10603cfb2 - __ZN4core6result13unwrap_failed17ha1863c6daffbf8fcE
  │    8:        0x105b25ed8 - __ZN6script3dom6window6Window12force_reflow17he22d23ffcb17d31eE
  │    9:        0x105b2657f - __ZN6script3dom6window6Window6reflow17hfc91aef0af84f7d8E
  │   10:        0x105dd3ded - __ZN6script3dom8document8Document11finish_load17h4bafdbb4a417d8e3E
  │   11:        0x105dfc57a - __ZN6script3dom11servoparser11ServoParser13do_parse_sync17h802bf791c7eabc4eE
  │   12:        0x10600e18d - __ZN14profile_traits4time7profile17h427d035c1d9226a4E
  │   13:        0x105dfbe7d - __ZN6script3dom11servoparser11ServoParser10parse_sync17he2aacf04fe38c77eE.llvm.3234429803088062424
  │   14:        0x105e00131 - __ZN93_$LT$script..dom..servoparser..ParserContext$u20$as$u20$net_traits..FetchResponseListener$GT$20process_response_eof17h5930a48aa7e6e197E
  │   15:        0x105fd4918 - __ZN6script13script_thread12ScriptThread29handle_msg_from_constellation17hd8a6d4c892cd2fdcE
  │   16:        0x105fccf25 - __ZN6script13script_thread12ScriptThread11handle_msgs17h45cd564a5b1e34e4E.llvm.5036957374981340486
  │   17:        0x106114d17 - __ZN14profile_traits3mem12ProfilerChan25run_with_memory_reporting17h3d92e242127af71eE
  │   18:        0x105f1c9c0 - __ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17hfd95f9d1f4f4dc99E
  │   19:        0x1059eea2a - __ZN3std9panicking3try7do_call17hf7e667a74b3a6f0dE.llvm.6741306198291749903
  │   20:        0x1081ae41e - ___rust_maybe_catch_panic
  │   21:        0x10603eee6 - __ZN50_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$8call_box17h970eaad3344c814dE
  │   22:        0x1081ad86b - __ZN3std3sys4unix6thread6Thread3new12thread_start17h2bafaa0015c96e6eE
  │   23:     0x7fff651ef660 - __pthread_body
  │   24:     0x7fff651ef50c - __pthread_start
  │ [2019-02-26T20:00:47Z ERROR servo] Layout thread disconnected.: "SendError(..)"
  └ Pipeline failed in hard-fail mode.  Crashing!

That is a different taskcluster failure, and looks unrelated to this PR.

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

💣 Failed to start rebuilding: Unknown error

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

⌛️ Testing commit c908701 with merge a810b35...

bors-servo added a commit that referenced this pull request Feb 27, 2019

Auto merge of #22938 - asajeffrey:webvr-future-frame-data, r=paulrouget
Use webvr future_frame_data to avoid blocking the WebGL thread

<!-- Please describe your changes on the following line: -->
This PR fixes a potential deadlock caused by the WebGL thread being blocked on a VR device. Rather than blocking on the VR device, it forwards a future to the script thread, and then then script thread blocks.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because it's fixing a potential deadlock

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22938)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

💔 Test failed - status-taskcluster

@CYBAI

This comment has been minimized.

Copy link
Collaborator

CYBAI commented Feb 27, 2019

@bors-servo retry

  • Timeout
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

💣 Failed to start rebuilding: Unknown error

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

⌛️ Testing commit c908701 with merge b01a4cc...

bors-servo added a commit that referenced this pull request Feb 27, 2019

Auto merge of #22938 - asajeffrey:webvr-future-frame-data, r=paulrouget
Use webvr future_frame_data to avoid blocking the WebGL thread

<!-- Please describe your changes on the following line: -->
This PR fixes a potential deadlock caused by the WebGL thread being blocked on a VR device. Rather than blocking on the VR device, it forwards a future to the script thread, and then then script thread blocks.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because it's fixing a potential deadlock

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22938)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

💔 Test failed - linux-rel-wpt

@CYBAI

This comment has been minimized.

Copy link
Collaborator

CYBAI commented Feb 27, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

💣 Failed to start rebuilding: Unknown error

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

⌛️ Testing commit c908701 with merge ec7a5f2...

bors-servo added a commit that referenced this pull request Feb 27, 2019

Auto merge of #22938 - asajeffrey:webvr-future-frame-data, r=paulrouget
Use webvr future_frame_data to avoid blocking the WebGL thread

<!-- Please describe your changes on the following line: -->
This PR fixes a potential deadlock caused by the WebGL thread being blocked on a VR device. Rather than blocking on the VR device, it forwards a future to the script thread, and then then script thread blocks.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because it's fixing a potential deadlock

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22938)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 27, 2019

@bors-servo bors-servo merged commit c908701 into servo:master Feb 27, 2019

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details

@bors-servo bors-servo referenced this pull request Feb 27, 2019

Open

Webgl profiling #22130

4 of 5 tasks complete
@asajeffrey

This comment has been minimized.

Copy link
Member Author

asajeffrey commented Feb 27, 2019

@CYBAI thanks for pushing this through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.