Re: DCHECK error in `BytesConsumer::GetError`

Clemens Backes

unread,

Jun 21, 2023, 9:46:46 AM6/21/23

to loadi...@chromium.org, Jakob Kummerow, Andreas Haas

Sending again after joining the group.

On Wed, Jun 21, 2023 at 3:40 PM Clemens Backes <clem...@chromium.org> wrote:

Hello loading team,

we (V8 future web / WebAssembly team) see a weird DCHECK error in the wild (on DCHECK-enabled canary builds) that we can't explain.

In our overridden `BytesConsumer::Client::OnStateChange()` method we get a `BytesConsumer::Result::kError` result when trying to read from the underlying `BytesConsumer`.
We then explicitly check the result of `BytesConsumer::GetPublicState()` (in an attempt to avoid the DCHECK), and get `kErrored`. The next thing we do is calling `BytesConsumer::GetError()`, but that fails with a DCHECK because the internal state is not `kErrored`.

The relevant code is here: https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc;l=254;drc=3e67c0491457f1a05fb7816346fc6d9676bb7a16

The `consumer_` is a `PlaceHolderBytesConsumer` holding a `DataPipeBytesConsumer`. If I read the code correctly, the only way to get back a public state of `kErrored` is if the `DataPipeBytesConsumer` has an internal state of `kErrored`. But then it should be possible to call `GetError()` on it.

Any ideas what could go wrong here?

See https://crbug.com/1449546 for links to the respective crashes.

Thanks in advance for any pointers!

-Clemens

--
Clemens Backes
Software Engineer
clem...@google.com
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls sie diese fälschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.

--

Clemens Backes

Software Engineer

clem...@google.com

Google Germany GmbH

Erika-Mann-Straße 33

80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls sie diese fälschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.

Yoshisato Yanagisawa

unread,

Jun 22, 2023, 2:36:07 AM6/22/23

to Clemens Backes, hiro...@chromium.org, loadi...@chromium.org, Jakob Kummerow, Andreas Haas

Sorry for the slow reply,

I took a look at the code, but unfortunately, I could not find anything more than you found.

The behavior cannot be explained except that either `underlying_` in `PlaceHolderBytesConsumer` or `state_` in `DataPipeBytesConsumer` was modified between L254 and L260, which should be unlikely.

According to the debug information, `state_` at crash time looks kWaiting, which is only set at the initialization. error_ seems to be an initial value. Therefore, I assume `GetPublicState()` somewhere else was called, but I am not sure how it is possible.

I am sorry I cannot help you.

+Hiroshige Will you take another look at this?

2023年6月21日(水) 22:46 Clemens Backes <clem...@chromium.org>:

--
You received this message because you are subscribed to the Google Groups "loading-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to loading-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/loading-dev/CAGO%3DqhAyVaxZf1Mv0gAyPqT%2BUyNJt1d53OyJM2e7YDGMUiKDOA%40mail.gmail.com.

Hiroshige Hayashizaki

unread,

Jun 22, 2023, 6:31:07 PM6/22/23

to Yoshisato Yanagisawa, Clemens Backes, loadi...@chromium.org, Jakob Kummerow, Andreas Haas

My initial feeling is that some other BytesConsumer is involved (e.g. ResponseBodyLoader::DelegatingBytesConsumer) while the class name didn't appear on the stack trace due to compiler optimizations.

https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/blink/renderer/platform/loader/fetch/response_body_loader.cc;l=140;drc=afec9eaf1d11cc77e8e06f06cb026fadf0dbf758;bpv=1;bpt=1

Clemens Backes

unread,

Jun 23, 2023, 7:00:14 AM6/23/23

to Hiroshige Hayashizaki, yoav...@chromium.org, Yoshisato Yanagisawa, Clemens Backes, loadi...@chromium.org, Jakob Kummerow, Andreas Haas

On Fri, Jun 23, 2023 at 12:31 AM Hiroshige Hayashizaki <hiro...@chromium.org> wrote:

My initial feeling is that some other BytesConsumer is involved (e.g. ResponseBodyLoader::DelegatingBytesConsumer) while the class name didn't appear on the stack trace due to compiler optimizations.
https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/blink/renderer/platform/loader/fetch/response_body_loader.cc;l=140;drc=afec9eaf1d11cc77e8e06f06cb026fadf0dbf758;bpv=1;bpt=1

Interesting theory, that could indeed be the case. It looks like the ResponseBodyLoader::DelegatingBytesConsumer should be fixed anyway to return an error if in errored state according to GetPublicState().

In order to learn more about the involved BytesConsumers, I uploaded two CLs to improve their DebugName() and to log it in a crash key: https://crrev.com/c/4640004 and https://crrev.com/c/4640242.

I'll report back in a few days when we see crashes on newer versions.

+yoav...@chromium.org for context.

Clemens Backes

unread,

Jun 28, 2023, 6:46:58 AM6/28/23

to Clemens Backes, Hiroshige Hayashizaki, yoav...@chromium.org, Yoshisato Yanagisawa, loadi...@chromium.org, Jakob Kummerow, Andreas Haas

On Fri, Jun 23, 2023 at 12:59 PM Clemens Backes <clem...@chromium.org> wrote:

On Fri, Jun 23, 2023 at 12:31 AM Hiroshige Hayashizaki <hiro...@chromium.org> wrote:
My initial feeling is that some other BytesConsumer is involved (e.g. ResponseBodyLoader::DelegatingBytesConsumer) while the class name didn't appear on the stack trace due to compiler optimizations.
https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/blink/renderer/platform/loader/fetch/response_body_loader.cc;l=140;drc=afec9eaf1d11cc77e8e06f06cb026fadf0dbf758;bpv=1;bpt=1

Interesting theory, that could indeed be the case. It looks like the ResponseBodyLoader::DelegatingBytesConsumer should be fixed anyway to return an error if in errored state according to GetPublicState().

Indeed, a DelegatingBytesConsumer is involved here, so this is probably the root cause.

We got a first report from a version containing the two CLs below, and the bytes consumer is a "PlaceHolderBytesConsumer(BufferingBytesConsumer(DelegatingBytesConsumer(DataPipeBytesConsumer)))".

I'll upload another CL to make DelegatingBytesConsumer::GetError return something if the loader is aborted.

Reply all

Reply to author

Forward