-[MTLCommandBuffer commit] may be called while encoding is still in progress #28680

mwyrzykowski · 2024-05-16T21:33:39Z

`03357c1`

-[MTLCommandBuffer commit] may be called while encoding is still in progress
https://bugs.webkit.org/show_bug.cgi?id=274275
<radar://128201828>

Reviewed by Dan Glastonbury.

Ensure we don't leave encoders open or commit command buffers
before encoding has ended.

* LayoutTests/TestExpectations:
* LayoutTests/fast/webgpu/fuzz-274275-expected.txt: Added.
* LayoutTests/fast/webgpu/fuzz-274275.html: Added.
Add regression test.

* Source/WebGPU/WebGPU/CommandEncoder.mm:
(WebGPU::CommandEncoder::endEncoding):
(WebGPU::CommandEncoder::runClearEncoder):
* Source/WebGPU/WebGPU/PresentationContextIOSurface.mm:
* Source/WebGPU/WebGPU/Queue.h:
* Source/WebGPU/WebGPU/Queue.mm:
(WebGPU::Queue::~Queue):
(WebGPU::Queue::ensureBlitCommandEncoder):
(WebGPU::Queue::finalizeBlitCommandEncoder):
(WebGPU::Queue::endEncoding const):
(WebGPU::Queue::setEncoderForBuffer):
Centralize endEncoding calls.

Canonical link: https://commits.webkit.org/279008@main

e5bb4b0

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-05-16T21:36:49Z

EWS run on previous version of this PR (hash e6abd5f)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
~~🧪 style~~	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 wincairo~~
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🧪 wpe-wk2
~~🧪 webkitperl~~	⏳ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🧪 api-wpe
	⏳ 🧪 ios-wk2-wpt	~~🧪 mac-wk1~~	⏳ 🛠 wpe-skia
	⏳ 🧪 api-ios	~~🧪 mac-wk2~~	✅ 🛠 gtk
	~~🛠 tv~~	⏳ 🧪 mac-AS-debug-wk2	⏳ 🧪 gtk-wk2
	~~🛠 tv-sim~~	~~🧪 mac-wk2-stress~~	⏳ 🧪 api-gtk
	~~🛠 watch~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2024-05-16T21:55:40Z

EWS run on previous version of this PR (hash 383923a)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
🧪 webkitpy	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🛠 watch
	✅ 🛠 watch-sim

…rogress WebKit#28680 <radar://128202303> Reviewed by NOBODY (OOPS!). ensureBlitCommandEncoder needs to ensure the encoding hasn't ended otherwise a new blit command encoder needs to be created. Depends on WebKit#28680 for the test to pass. * LayoutTests/TestExpectations: * LayoutTests/fast/webgpu/fuzz-274289-expected.txt: Added. * LayoutTests/fast/webgpu/fuzz-274289.html: Added. Add regression test. * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::Queue::ensureBlitCommandEncoder):

https://bugs.webkit.org/show_bug.cgi?id=274289 <radar://128202303> Reviewed by NOBODY (OOPS!). ensureBlitCommandEncoder needs to ensure the encoding hasn't ended otherwise a new blit command encoder needs to be created. Depends on WebKit#28680 for the test to pass. * LayoutTests/TestExpectations: * LayoutTests/fast/webgpu/fuzz-274289-expected.txt: Added. * LayoutTests/fast/webgpu/fuzz-274289.html: Added. Add regression test. * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::Queue::ensureBlitCommandEncoder):

djg

djg · 2024-05-18T08:11:35Z

Source/WebGPU/WebGPU/Queue.mm

+{
+    id<MTLCommandEncoder> currentEncoder = encoderForBuffer(commandBuffer);
+    if (currentEncoder != commandEncoder)
+        return;


Is it an error to try close the non-current encoder? Should it be?

Not currently because we automatically stop encoding when the website attempts to leave more than 64 open encoders.

It would seem we should never reach this if we remove that logic dependent on the outcome of gpuweb/gpuweb#4622

webkit-early-warning-system · 2024-05-20T17:22:00Z

EWS run on current version of this PR (hash e5bb4b0)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

…rogress https://bugs.webkit.org/show_bug.cgi?id=274275 <radar://128201828> Reviewed by Dan Glastonbury. Ensure we don't leave encoders open or commit command buffers before encoding has ended. * LayoutTests/TestExpectations: * LayoutTests/fast/webgpu/fuzz-274275-expected.txt: Added. * LayoutTests/fast/webgpu/fuzz-274275.html: Added. Add regression test. * Source/WebGPU/WebGPU/CommandEncoder.mm: (WebGPU::CommandEncoder::endEncoding): (WebGPU::CommandEncoder::runClearEncoder): * Source/WebGPU/WebGPU/PresentationContextIOSurface.mm: * Source/WebGPU/WebGPU/Queue.h: * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::Queue::~Queue): (WebGPU::Queue::ensureBlitCommandEncoder): (WebGPU::Queue::finalizeBlitCommandEncoder): (WebGPU::Queue::endEncoding const): (WebGPU::Queue::setEncoderForBuffer): Centralize endEncoding calls. Canonical link: https://commits.webkit.org/279008@main

webkit-commit-queue · 2024-05-20T19:56:43Z

Committed 279008@main (03357c1): https://commits.webkit.org/279008@main

Reviewed commits have been landed. Closing PR #28680 and removing active labels.

https://bugs.webkit.org/show_bug.cgi?id=274289 <radar://128202303> Reviewed by NOBODY (OOPS!). ensureBlitCommandEncoder needs to ensure the encoding hasn't ended otherwise a new blit command encoder needs to be created. Depends on WebKit#28680 for the test to pass. * LayoutTests/TestExpectations: * LayoutTests/fast/webgpu/fuzz-274289-expected.txt: Added. * LayoutTests/fast/webgpu/fuzz-274289.html: Added. Add regression test. * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::Queue::ensureBlitCommandEncoder):

https://bugs.webkit.org/show_bug.cgi?id=274289 <radar://128202303> Reviewed by Dan Glastonbury. ensureBlitCommandEncoder needs to ensure the encoding hasn't ended otherwise a new blit command encoder needs to be created. Depends on WebKit#28680 for the test to pass. * LayoutTests/TestExpectations: * LayoutTests/fast/webgpu/fuzz-274289-expected.txt: Added. * LayoutTests/fast/webgpu/fuzz-274289.html: Added. Add regression test. * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::Queue::ensureBlitCommandEncoder): Canonical link: https://commits.webkit.org/279018@main

https://bugs.webkit.org/show_bug.cgi?id=274289 <radar://128202303> Reviewed by Dan Glastonbury. ensureBlitCommandEncoder needs to ensure the encoding hasn't ended otherwise a new blit command encoder needs to be created. Depends on WebKit#28680 for the test to pass. * LayoutTests/TestExpectations: * LayoutTests/fast/webgpu/fuzz-274289-expected.txt: Added. * LayoutTests/fast/webgpu/fuzz-274289.html: Added. Add regression test. * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::Queue::ensureBlitCommandEncoder): Canonical link: https://commits.webkit.org/279026@main

mwyrzykowski requested a review from tadeuzagallo as a code owner May 16, 2024 21:33

mwyrzykowski self-assigned this May 16, 2024

mwyrzykowski added the WebGPU For bugs in WebGPU label May 16, 2024

mwyrzykowski force-pushed the eng/-MTLCommandBuffer-commit-may-be-called-while-encoding-is-still-in-progress branch from e6abd5f to 383923a Compare May 16, 2024 21:53

mwyrzykowski mentioned this pull request May 17, 2024

[WebGPU] Queue may try to encode blit operations on detached encoder #28688

Merged

mwyrzykowski requested a review from djg May 17, 2024 23:19

djg approved these changes May 18, 2024

View reviewed changes

mwyrzykowski force-pushed the eng/-MTLCommandBuffer-commit-may-be-called-while-encoding-is-still-in-progress branch from 383923a to e5bb4b0 Compare May 20, 2024 17:21

mwyrzykowski added the merge-queue Applied to send a pull request to merge-queue label May 20, 2024

webkit-commit-queue force-pushed the eng/-MTLCommandBuffer-commit-may-be-called-while-encoding-is-still-in-progress branch from e5bb4b0 to 6860782 Compare May 20, 2024 19:55

webkit-commit-queue force-pushed the eng/-MTLCommandBuffer-commit-may-be-called-while-encoding-is-still-in-progress branch from 6860782 to 03357c1 Compare May 20, 2024 19:56

webkit-commit-queue merged commit 03357c1 into WebKit:main May 20, 2024

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

-[MTLCommandBuffer commit] may be called while encoding is still in progress #28680

-[MTLCommandBuffer commit] may be called while encoding is still in progress #28680

mwyrzykowski commented May 16, 2024 •

edited by webkit-early-warning-system

webkit-early-warning-system commented May 16, 2024 •

edited

webkit-early-warning-system commented May 16, 2024 •

edited

djg left a comment

djg May 18, 2024

mwyrzykowski May 20, 2024

webkit-early-warning-system commented May 20, 2024 •

edited

webkit-commit-queue commented May 20, 2024

-[MTLCommandBuffer commit] may be called while encoding is still in progress #28680

-[MTLCommandBuffer commit] may be called while encoding is still in progress #28680

Conversation

mwyrzykowski commented May 16, 2024 • edited by webkit-early-warning-system

03357c1

webkit-early-warning-system commented May 16, 2024 • edited

webkit-early-warning-system commented May 16, 2024 • edited

djg left a comment

Choose a reason for hiding this comment

djg May 18, 2024

Choose a reason for hiding this comment

mwyrzykowski May 20, 2024

Choose a reason for hiding this comment

webkit-early-warning-system commented May 20, 2024 • edited

webkit-commit-queue commented May 20, 2024

mwyrzykowski commented May 16, 2024 •

edited by webkit-early-warning-system

`03357c1`

webkit-early-warning-system commented May 16, 2024 •

edited

webkit-early-warning-system commented May 16, 2024 •

edited

webkit-early-warning-system commented May 20, 2024 •

edited