[wasm][shared] Fix cross-instance type checks [v8/v8 : main]
0 views
Skip to first unread message
Manos Koukoutos (Gerrit)
Jul 23, 2026, 11:17:50 AM (2 days ago) Jul 23
to Jakob Kummerow, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Jakob Kummerow
Manos Koukoutos voted and added 1 comment![]( "Open in Gerrit")
Votes added by Manos Koukoutos
| Commit-Queue | +1 |
1 comment
File src/wasm/module-instantiate.cc
Line 75, Patchset 2 (Latest):
shared_space_isolate->wasm_shared_canonical_types_mutex()->Lock();Manos Koukoutos . unresolved
I did not find a nice way to use `MutexGuard` here.
Related details
Attention is currently required from:
- Jakob Kummerow
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Kummerow (Gerrit)
Jul 24, 2026, 6:23:16 AM (23 hours ago) Jul 24
to Manos Koukoutos, Jakob Kummerow, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Manos Koukoutos
Jakob Kummerow voted and added 3 comments![]( "Open in Gerrit")
Votes added by Jakob Kummerow
| Code-Review | +1 |
3 comments
Patchset-level comments
File src/wasm/canonical-types.cc
Line 583, Patchset 3 (Latest):
MaybeDirectHandle<WeakFixedArray> maybe_new_rtts =Jakob Kummerow . unresolved
You could consider inlining this for brevity:
```
if (MaybeGrowRtts(...).ToHandle(&new_rtts)) {
...
```
at the cost of having more stuff on that `if` line. Up to you; we have precedent for both patterns.
File src/wasm/module-instantiate.cc
Line 75, Patchset 2:
shared_space_isolate->wasm_shared_canonical_types_mutex()->Lock();Manos Koukoutos . unresolved
I did not find a nice way to use `MutexGuard` here.
Jakob Kummerow
`std::optional<base::MutexGuard> lock;` near line 72, and `lock.emplace(..._mutex())` here.
Related details
Attention is currently required from:
- Manos Koukoutos
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manos Koukoutos (Gerrit)
Jul 24, 2026, 9:28:43 AM (20 hours ago) Jul 24
to Dominik Inführ, Jakob Kummerow, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Dominik Inführ
Manos Koukoutos voted and added 3 comments![]( "Open in Gerrit")
Votes added by Manos Koukoutos
| Commit-Queue | +1 |
3 comments
Patchset-level comments
File src/wasm/canonical-types.cc
You could consider inlining this for brevity:
```
if (MaybeGrowRtts(...).ToHandle(&new_rtts)) {
...
```
at the cost of having more stuff on that `if` line. Up to you; we have precedent for both patterns.
Manos Koukoutos
Done
File src/wasm/module-instantiate.cc
Line 75, Patchset 2:
shared_space_isolate->wasm_shared_canonical_types_mutex()->Lock();Manos Koukoutos . resolved
Jakob KummerowI did not find a nice way to use `MutexGuard` here.
`std::optional<base::MutexGuard> lock;` near line 72, and `lock.emplace(..._mutex())` here.
Attention is currently required from:
- Dominik Inführ
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Jul 24, 2026, 9:35:34 AM (19 hours ago) Jul 24
to Manos Koukoutos, Jakob Kummerow, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Manos Koukoutos
Dominik Inführ voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominik Inführ
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Manos Koukoutos
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manos Koukoutos (Gerrit)
Jul 24, 2026, 9:49:17 AM (19 hours ago) Jul 24
to Dominik Inführ, Jakob Kummerow, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Manos Koukoutos voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
v8-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
Jul 24, 2026, 10:10:46 AM (19 hours ago) Jul 24
to Manos Koukoutos, Dominik Inführ, Jakob Kummerow, Hannes Payer, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
v8-s...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[wasm][shared] Fix cross-instance type checks
Shared rtts have to be shared across all instances defining the same
canonical type. Therefore we need to maintain an array of them in the
shared-space isolate. Note that we cannot just store them in the
shared-space isolate's wasm_canonical_rtts array because that is not
shared and therefore is not accessible by other isolates.
Bug: 42204563
Change-Id: I32e6da79e2273e9405db5953cad020de23826c56
Reviewed-by: Jakob Kummerow <jkum...@chromium.org>
Reviewed-by: Dominik Inführ <dinf...@chromium.org>
Commit-Queue: Manos Koukoutos <mano...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#108868}
Files:
- M src/execution/isolate.h
- M src/heap/heap-inl.h
- M src/heap/heap.h
- M src/heap/setup-heap-internal.cc
- M src/roots/roots.h
- M src/wasm/canonical-types.cc
- M src/wasm/canonical-types.h
- M src/wasm/module-instantiate.cc
- M test/common/wasm/fuzzer-common.cc
- M test/common/wasm/wasm-run-utils.cc
- A test/mjsunit/wasm/shared-everything/ref-cast-in-different-isolate.js
- M test/unittests/objects/roots-unittest.cc
Change size: M
Delta: 12 files changed, 151 insertions(+), 21 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Jakob Kummerow, +1 by Dominik Inführ
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manos Koukoutos (Gerrit)
Jul 24, 2026, 12:18:47 PM (17 hours ago) Jul 24
to v8-s...@luci-project-accounts.iam.gserviceaccount.com, Dominik Inführ, Jakob Kummerow, Hannes Payer, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Manos Koukoutos has created a revert of this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages