Skip to content

Add early-exit validation to MapAsync for already mapped buffers and … #42

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rajv79
Copy link

@rajv79 rajv79 commented Jul 18, 2025

Summary of Commit

This PR adds an early-exit check in BufferBase::APIMapAsync() to prevent calling
MapAsync on a buffer that is already mapped (either via MapAsync or mappedAtCreation).

It improves developer feedback and avoids redundant validation, while returning
a null future and triggering the error callback early.

Key Changes

  • Checks the current buffer state before ValidateMapAsync()
  • Emits a warning using dawn::EmitWarning
  • Returns a null Future on invalid use
  • Invokes the callback with WGPUMapAsyncStatus_Error

Test Coverage

Added MapAsyncFailsWhenAlreadyMapped in BufferValidationTests to verify behavior.

CI Note (Temporary Workaround)

This PR includes a temporary local patch to work around a known Protobuf build issue on macOS
It can be removed once the CI or Protobuf dependency is updated.

Copy link

google-cla bot commented Jul 18, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@rajv79 rajv79 force-pushed the fix-buffer-mapasync-double-call branch from 1f3e7cd to f601680 Compare July 18, 2025 06:55
Copy link

👋 Thanks for your contribution! Your PR has been imported to Gerrit.
Please visit https://dawn-review.googlesource.com/c/dawn/+/253674 to see it and CC yourself on the change.
After iterating on feedback, please comment on the Gerrit review to notify reviewers.
All reviews are handled within Gerrit, any comments on the GitHub PR may be missed.
You can continue to upload commits to this PR, and they will be automatically imported
into Gerrit.

@rajv79
Copy link
Author

rajv79 commented Jul 18, 2025

Hi @nico @precision @dj2 ,
I tested the change locally, and it worked. However, the CI is failing with a build error related to Protobuf and constinit, which I wasn't expecting. After looking into it, I found it’s a known issue on macOS with Xcode/Clang.

To help CI proceed, I temporarily disabled DAWN_ENABLE_INSTALL in the cache config file. It doesn’t affect the feature I added — just helps bypass the CMake export error.

Please let me know if there’s a better way to handle this. Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant