New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 829659 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Autodesk Crash in M66 (but not M67)

Project Member Reported by bradnelson@chromium.org, Apr 6 2018

Issue description

Crash report:
https://crash.corp.google.com/browse?stbtiq=42338fb07cea8350

Magic SignatureIPC::ChannelProxy::SendInternal
(crbug 813471, crbug 766713, crbug 766691)
User bug(s): crbug 784069, crbug 796354

 
Ken, how confident are we this is an instance of:
https://bugs.chromium.org/p/chromium/issues/detail?id=784069

Thanks!
I don't know what I'm supposed to do with this though

Comment 4 by creis@chromium.org, Apr 10 2018

Cc: dcheng@chromium.org lukasza@chromium.org
bradnelson@: Can you provide some more context on this bug?  Are you saying that loading Autodesk caused a crash in this particular way, presumably with Site Isolation enabled?  What are the repro steps?

The crash report is from an attempt to send a postMessage from a frame in one process to a frame in another process.  Given Ken's link in comment 2, this suggests that the postMessage IPC was somehow deemed "oversized," which caused the renderer to crash.

What's the postMessage being sent here, and how big is it?  rockot@, is this size limit new, or did we have one before?
We sent many messages into an iframe, and from the iframe document to a worker. Is there a way to isolate on your side what mesaage it was? Was it to the iframe? To the worker? The binary data is 6mb, not sure once we serialize it. We also do software rendering in the worker and pass it back, that could be of significant size. If you have the data in the stack trace perhaps we can ellucidate?

Comment 6 by creis@chromium.org, Apr 10 2018

I don't have that data in the crash dump-- just an indication that a frame was sending a postMessage to another process (probably to the worker, given your description).  It does sound like the messages in your app are quite large, by design.

That said, there's no repro steps on this bug report, so I don't have other context.  Can you provide repro steps for us to try?

At first glance, I'm kind of surprised we're imposing a size limit on postMessage.  rockot@, can you comment more on why there's a limit?  If we had one before, was it higher than what we have now?
We can provide with a username and password and steps tomorrow. One detail is that this specific issue I believe it happened in Chrome beta, not Canary. Is it not reported as such?

Comment 8 by roc...@chromium.org, Apr 10 2018

Re #6: We've always had a limit on IPC message length, and it's always been 128 MB, as it is today. The difference is that before we used to treat these messages as bad IPCs on the browser side (and shoot the sending renderer) whereas now we CHECK fail in the sender (more useful crash reports).

Very large IPCs are bad for performance due to both to excessive copying as well as contention on the IPC channel.

The good news is that it should now be pretty easy to change message port's CloneableMessage[1] structure to use a BigBuffer[2] instead of an inlined byte array. This will use either an inline byte array OR a shared memory region depending on how large the data is at message construction time.

In the meantime, I would recommend that large postMessage messages be chunked by the application.

[1] https://cs.chromium.org/chromium/src/third_party/blink/public/mojom/message_port/message_port.mojom?rcl=0aadd1504792c8dc07bd46c0f4493f5630c2c1e0&l=19
[2] https://cs.chromium.org/chromium/src/mojo/public/mojom/base/big_buffer.mojom?rcl=0aadd1504792c8dc07bd46c0f4493f5630c2c1e0&l=15

Comment 9 by roc...@chromium.org, Apr 10 2018

Also to clarify, there is nothing special about postMessage in this regard. It's always been implemented as a single IPC message and has always been subject to these constraints.
Cc: lafo...@chromium.org
Summary: Autodesk Crash in M66 (but not M67) (was: Autodesk Crash in Beta (also happening in Canary))
Ken, Autodesk is seeing this with Autocad when loading largish CAD files. Their application is making fairly heavy use of postMessage at the moment as they had been planning to rely on SharedArrayBuffers, but have worked around it temporarily. They're using postMessage both to send CAD files to the core of their application which runs in WebAssembly in a Worker, as well as to send render data back from that worker to the main thread for rendering.

Martin, Kevin, are these large postMessages sent as strings, ArrayBuffer, or Blobs?
Ken, in terms of the the limit they're hitting, do any of the 3 forms make a difference in terms of being chunked, or are single postMessages always 1 IPC underneath?

In terms of crashes I can find, they all appear to be in M66 for this exact crash, and others through the same CHECK in M67 seem to be unrelated (things like devtools).

Ken, any theory why they'd be seeing this only in Beta?

A single postMessage call is always one IPC regardless of message composition.

If these messages aren't actually crossing a process boundary -- it sounds like they aren't -- this crash is almost certainly (and inadvertently) "fixed" by malcolmwhite@'s work to use lazy serialization for message ports.
Hmm maybe not. I would expect this change to be responsible: https://chromium-review.googlesource.com/c/chromium/src/+/854240

but that made the cut for M66.
Anyway, if messages of roughly the same size are crashing in M66 but not M67, it probably does mean the messages are -- for whatever reason -- not actually crossing a process boundary anymore in M67. Message size limits have not changed.

I still think the correct fix is to change skia.mojom.Bitmap, blink.mojom.ArrayBufferContents, and blink.mojom.CloneableMessage.encoded_message to use a BigBuffer instead of array<uint8>. Otherwise applications posting large messages will always be dancing around this issue.

I'm not sure what we can realistically do about M66 though.
Thanks Ken.

Kevin, Martin, eager for a repro in hand, as that may help. As would better understanding the size + number + composition of the messages you're sending.

I am sorry but we don’t have repro steps for this one. I will try tomorrow see if I can get something for you guys. 
FWIW I have a few patches out to address this. Unfortunately I think
they're a bit too complex to justify merging back, maybe even to 67, let
alone 66.
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb

commit 1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Apr 12 05:32:55 2018

Use BigBuffer for MessagePort message contents

Switches ArrayBuffer and raw encoded message data mojom representations
to use BigBuffer instead of array<uint8>. This avoids large message data
exceeding hard size limitations.

Also introduces BigBufferView as a traits helper to avoid redundant
copies when a deserialized BigBuffer is using inline array storage.

Bug: 829659
Change-Id: I8b62d4bcd7458db1d6aff9e22dcb7c13999f0bf0
Reviewed-on: https://chromium-review.googlesource.com/1004594
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550021}
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/content/public/android/BUILD.gn
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/content/public/android/java/src/org/chromium/content/browser/AppWebMessagePort.java
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/mojo/public/cpp/base/big_buffer.cc
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/mojo/public/cpp/base/big_buffer.h
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/mojo/public/cpp/base/big_buffer_mojom_traits.cc
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/mojo/public/cpp/base/big_buffer_mojom_traits.h
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/mojo/public/java/BUILD.gn
[add] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/mojo/public/java/base/src/org/chromium/mojo_base/BigBufferUtil.java
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/WebKit/Tools/Scripts/audit-non-blink-usage.py
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/common/message_port/cloneable_message_struct_traits.cc
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/common/message_port/cloneable_message_struct_traits.h
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/common/message_port/transferable_message_struct_traits.h
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/public/mojom/BUILD.gn
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/public/mojom/array_buffer/array_buffer_contents.mojom
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/public/mojom/message_port/message_port.mojom
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/renderer/core/DEPS
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/renderer/core/messaging/blink_cloneable_message_struct_traits.cc
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/renderer/core/messaging/blink_cloneable_message_struct_traits.h
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/renderer/core/messaging/blink_transferable_message_struct_traits.cc
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/renderer/core/messaging/blink_transferable_message_struct_traits.h
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/renderer/core/messaging/blink_transferable_message_struct_traits_test.cc
[modify] https://crrev.com/1ca393124cd1c2c6ecd17bc94e17a8be72bc0bbb/third_party/blink/renderer/core/messaging/message_port.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/552293aab99d28ec9592c967bd12a5332d9b2ce5

commit 552293aab99d28ec9592c967bd12a5332d9b2ce5
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Apr 17 05:19:36 2018

Use BigBuffer for SkBitmap mojom pixel data

This avoids large bitmaps yielding large messages, as BigBuffer falls
back onto shared memory beyond a certain size threshold.

Bug: 829659
Cq-Include-Trybots: luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win10_chromium_x64_rel_ng
Change-Id: I82f907144944f2e8ebea51264fc00afc171ce3bf
Reviewed-on: https://chromium-review.googlesource.com/1004598
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551254}
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/BUILD.gn
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/services/BUILD.gn
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/services/shape_detection/BUILD.gn
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/services/shape_detection/android/java/src/org/chromium/shape_detection/BitmapUtils.java
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/services/shape_detection/android/javatests/src/org/chromium/shape_detection/TestUtils.java
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/services/shape_detection/android/junit/src/org/chromium/shape_detection/BitmapUtilsTest.java
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/services/ui/public/cpp/property_type_converters.cc
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/skia/public/interfaces/BUILD.gn
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/skia/public/interfaces/bitmap.mojom
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/skia/public/interfaces/bitmap_skbitmap_struct_traits.cc
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/skia/public/interfaces/bitmap_skbitmap_struct_traits.h
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/skia/public/interfaces/skbitmap.typemap
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/detection-HTMLCanvasElement.html
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/detection-HTMLImageElement.html
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/detection-HTMLVideoElement.html
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/detection-ImageBitmap.html
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/detection-ImageData.html
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/detection-options.html
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/detection-security-test.html
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/detector-same-object.html
[add] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/resources/big-buffer-helpers.js
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/resources/mock-barcodedetection.js
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/resources/mock-facedetection.js
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/resources/mock-textdetection.js
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/WebKit/LayoutTests/shapedetection/resources/worker.js
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/third_party/blink/renderer/platform/mojo/notification_struct_traits_test.cc
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/ui/gfx/image/mojo/BUILD.gn
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/ui/gfx/image/mojo/image.mojom
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/ui/gfx/image/mojo/image.typemap
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/ui/gfx/image/mojo/image_skia_struct_traits.cc
[modify] https://crrev.com/552293aab99d28ec9592c967bd12a5332d9b2ce5/ui/gfx/image/mojo/image_skia_struct_traits.h

Comment 19 by creis@chromium.org, Apr 26 2018

Just checking-- do we think this is resolved after the commits in comments 17-18?  (I'm not sure I fully understand how everything connects, if the issue affects M66 and not M67 and we landed CLs in M68.  Are things in good shape on M67?)
Not merged to M67, so the problem still exists there. I suspect merging
would be pretty challenging and therefore risky.
We could not get repro steps for this issue, I am sorry. Based on the comments here, we will keep an eye on 67 (now beta, right?) and see if the issue occurs. We recently changed our IPC usage quite dramatically, so that could have an effect as well. Will this issue stay open until after 67 is released?

Comment 22 by mek@chromium.org, May 9 2018

Components: Blink>Messaging
Owner: rockot@google.com
Cc: -roc...@chromium.org rockot@google.com

Sign in to add a comment