Prevent crashes during isolate teardown and invalid V8 stack access. [chromium/src : main]

0 views
Skip to first unread message

Joone Hur (Gerrit)

unread,
Oct 24, 2025, 11:14:53 PM (8 days ago) Oct 24
to Andrey Kosyakov, Noam Rosenthal, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, Olga Gerchikov, Kentaro Hara, Raphael Kubo da Costa, blink-revie...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Andrey Kosyakov and Noam Rosenthal

Joone Hur added 1 comment

Patchset-level comments
File-level comment, Patchset 3 (Latest):
Joone Hur . resolved

Hi, I’ve added some defensive checks in LazySourceLocation and AnimationFrameTimingMonitor to address a crash issue.
I’d appreciate any feedback or suggestions.

Open in Gerrit

Related details

Attention is currently required from:
  • Andrey Kosyakov
  • Noam Rosenthal
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I36e07c14f1f2414d256370268b58590bc68ce364
Gerrit-Change-Number: 7083316
Gerrit-PatchSet: 3
Gerrit-Owner: Joone Hur <joon...@microsoft.com>
Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
Gerrit-Reviewer: Joone Hur <joon...@microsoft.com>
Gerrit-Reviewer: Noam Rosenthal <nrose...@google.com>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
Gerrit-Attention: Andrey Kosyakov <ca...@chromium.org>
Gerrit-Attention: Noam Rosenthal <nrose...@google.com>
Gerrit-Comment-Date: Sat, 25 Oct 2025 03:14:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Andrey Kosyakov (Gerrit)

unread,
Oct 27, 2025, 8:05:21 PM (6 days ago) Oct 27
to Joone Hur, Noam Rosenthal, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, Olga Gerchikov, Kentaro Hara, Raphael Kubo da Costa, blink-revie...@chromium.org, blink-revie...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Joone Hur and Noam Rosenthal

Andrey Kosyakov added 4 comments

File third_party/blink/renderer/core/frame/animation_frame_timing_monitor.cc
Line 644, Patchset 3 (Latest): if (!isolate || isolate->IsDead()) {
Andrey Kosyakov . unresolved

Why do we even still have this check? Thought we discussed this. No, isolate can't be null here, because it's never reset to null in ScriptState.

File third_party/blink/renderer/platform/bindings/lazy_source_location.cc
Line 20, Patchset 3 (Latest): v8::HandleScope handle_scope(isolate);
Andrey Kosyakov . unresolved

Not sure I see how this (or any other changes in this method) would be related to the crash in question. We already deal with Locals on the call site, so perhaps this is redundant?

Line 30, Patchset 3 (Latest): if (stack_frame.IsEmpty()) {
Andrey Kosyakov . unresolved

I don't think this would ever happen. Even if we did, we would just end up with a null dereference below, not with the kind of corruption that the original crash implies.

Line 36, Patchset 3 (Latest): if (script_name_v8.IsEmpty()) {
Andrey Kosyakov . unresolved

Why though? LazySourceLocation seems to be able to deal with an empty script url just fine.

Open in Gerrit

Related details

Attention is currently required from:
  • Joone Hur
  • Noam Rosenthal
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I36e07c14f1f2414d256370268b58590bc68ce364
    Gerrit-Change-Number: 7083316
    Gerrit-PatchSet: 3
    Gerrit-Owner: Joone Hur <joon...@microsoft.com>
    Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Reviewer: Joone Hur <joon...@microsoft.com>
    Gerrit-Reviewer: Noam Rosenthal <nrose...@google.com>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
    Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
    Gerrit-Attention: Joone Hur <joon...@microsoft.com>
    Gerrit-Attention: Noam Rosenthal <nrose...@google.com>
    Gerrit-Comment-Date: Tue, 28 Oct 2025 00:05:11 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages