Skip to content

fix: do not pass a DesktopMediaList* to DesktopCapturer::OnListReady()#51399

Merged
ckerr merged 1 commit into
mainfrom
fix/desktop-capturer-dangling-list-callback
May 1, 2026
Merged

fix: do not pass a DesktopMediaList* to DesktopCapturer::OnListReady()#51399
ckerr merged 1 commit into
mainfrom
fix/desktop-capturer-dangling-list-callback

Conversation

@ckerr

@ckerr ckerr commented Apr 30, 2026

Copy link
Copy Markdown
Member

Description of Change

Fix another main-only desktopCapturer regression.

In DesktopCapturer::OnListReady(DesktopMediaList* list), the list arg was just used as a proxy for the list type, so so just pass the type instead. This solves a lifecycle issue occurring in CI where the callack can outlive the DesktopMediaList.

Sample error log from https://github.com/electron/electron/actions/runs/25070468959/job/73456070492:

[48471:0428/193441.269750:FATAL:base/allocator/partition_alloc_support.cc:798] Detected dangling raw_ptr in unretained with id=0x0000013c02e14378:
 Task trace:
 0   Electron Framework  0x000000012283a0ba electron::api::DesktopCapturer::ListObserver::MaybeNotifyReady() + 170
 1   Electron Framework  0x0000000133246dc5 NativeDesktopMediaList::Worker::OnRecurrentCaptureResult(webrtc::DesktopCapturer::Result, std::__Cr::unique_ptr<webrtc::DesktopFrame, std::__Cr::default_delete<webrtc::DesktopFrame>>, long) + 357
 2   Electron Framework  0x000000013328dbcf (anonymous namespace)::ScreenshotManagerCapturer::OnRecurrentCaptureTimer() + 1343
 Stack trace:
 0   Electron Framework  0x000000012ade42f2 base::debug::CollectStackTrace(base::span<void const*, 18446744073709551615ul, void const**>) + 18
 1   Electron Framework  0x000000012add00e1 base::debug::StackTrace::StackTrace(unsigned long) + 225
 2   Electron Framework  0x000000012ade978a base::allocator::UnretainedDanglingRawPtrDetectedCrash(unsigned long) + 90
 3   Electron Framework  0x000000012ae437f7 base::internal::RawPtrBackupRefImpl<true>::ReportIfDanglingInternal(unsigned long) + 391

Checklist

Release Notes

Notes: none

@ckerr ckerr added semver/patch backwards-compatible bug fixes no-backport labels Apr 30, 2026
@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Apr 30, 2026

@dsanders11 dsanders11 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just need to drop the break after NOTREACHED() per the build errors.

…eady()

The list pointer was being used as a proxy for its type, so just pass
the type instead. This solves a lifecycle issue occurring in CI where
the callack can outlive the DesktopMediaList.

Sample error log:

[48471:0428/193441.269750:FATAL:base/allocator/partition_alloc_support.cc:798] Detected dangling raw_ptr in unretained with id=0x0000013c02e14378:
 Task trace:
 0   Electron Framework  0x000000012283a0ba electron::api::DesktopCapturer::ListObserver::MaybeNotifyReady() + 170
 1   Electron Framework  0x0000000133246dc5 NativeDesktopMediaList::Worker::OnRecurrentCaptureResult(webrtc::DesktopCapturer::Result, std::__Cr::unique_ptr<webrtc::DesktopFrame, std::__Cr::default_delete<webrtc::DesktopFrame>>, long) + 357
 2   Electron Framework  0x000000013328dbcf (anonymous namespace)::ScreenshotManagerCapturer::OnRecurrentCaptureTimer() + 1343
 Stack trace:
 0   Electron Framework  0x000000012ade42f2 base::debug::CollectStackTrace(base::span<void const*, 18446744073709551615ul, void const**>) + 18
 1   Electron Framework  0x000000012add00e1 base::debug::StackTrace::StackTrace(unsigned long) + 225
 2   Electron Framework  0x000000012ade978a base::allocator::UnretainedDanglingRawPtrDetectedCrash(unsigned long) + 90
 3   Electron Framework  0x000000012ae437f7 base::internal::RawPtrBackupRefImpl<true>::ReportIfDanglingInternal(unsigned long) + 391
@ckerr ckerr force-pushed the fix/desktop-capturer-dangling-list-callback branch from da75515 to ae232d2 Compare April 30, 2026 05:00
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label May 1, 2026
@ckerr ckerr merged commit f8d0412 into main May 1, 2026
69 checks passed
@release-clerk

release-clerk Bot commented May 1, 2026

Copy link
Copy Markdown

No Release Notes

@ckerr ckerr deleted the fix/desktop-capturer-dangling-list-callback branch May 1, 2026 16:14
ckerr added a commit that referenced this pull request May 6, 2026
* fix: timing issue DCHECK crash in DesktopCapturer on macOS (#50960)

refactor: use StartUpdating in desktopCapturer

Replace the one-shot Update() callback model with the continuous
StartUpdating() observer model for NativeDesktopMediaList.

Fixes a macOS DCHECK(can_refresh()) crash in UpdateSourceThumbnail(),
where ScreenCaptureKit's recurrent thumbnail capturer would post
UpdateSourceThumbnail callbacks after the one-shot refresh_callback_
had been consumed. Now, can_refresh() is always true because
refresh_callback_ is repopulated via ScheduleNextRefresh().

Each capturer (window, screen) gets its own ListObserver that tracks
readiness via OnSourceAdded and OnSourceThumbnailChanged events.
Once a list has both sources and thumbnails (or thumbnails aren't
requested), its data is snapshotted and the capturer checks if all
requested types are ready before resolving to JS.

Also remove the "skip_next_refresh_" Chromium patch, which was a
workaround for the timing mismatch between the one-shot Update()
model and ScreenCaptureKit's asynchronous thumbnail delivery.

refactor: simplify state logic in DesktopCapturer
(cherry picked from commit dad4ab6)

* fix: do not block indefinitely on thumbnails in desktopCapturer (#51128)

* fix: do not block indefinitely on thumbnails in desktopCapturer

fixes dad4ab6 regression

* fix: build error

* fixup! fix: do not block indefinitely on thumbnails in desktopCapturer

chore: remove unnecessary code

* Update shell/browser/api/electron_api_desktop_capturer.cc

Co-authored-by: Niklas Wenzel <dev@nikwen.de>

---------

Co-authored-by: Niklas Wenzel <dev@nikwen.de>
(cherry picked from commit a1d28e6)

* fix: dangling `raw_ptr` regression in `DesktopCapturer` (#51158)

fix: dangling raw_ptr in desktopCapturer
(cherry picked from commit bef68b6)

* fix: do not pass a `DesktopMediaList* to DesktopCapturer::OnListReady()` (#51399)

refactor: do not pass a DesktopMediaList* to DesktopCapturer::OnListReady()

The list pointer was being used as a proxy for its type, so just pass
the type instead. This solves a lifecycle issue occurring in CI where
the callack can outlive the DesktopMediaList.

Sample error log:

[48471:0428/193441.269750:FATAL:base/allocator/partition_alloc_support.cc:798] Detected dangling raw_ptr in unretained with id=0x0000013c02e14378:
 Task trace:
 0   Electron Framework  0x000000012283a0ba electron::api::DesktopCapturer::ListObserver::MaybeNotifyReady() + 170
 1   Electron Framework  0x0000000133246dc5 NativeDesktopMediaList::Worker::OnRecurrentCaptureResult(webrtc::DesktopCapturer::Result, std::__Cr::unique_ptr<webrtc::DesktopFrame, std::__Cr::default_delete<webrtc::DesktopFrame>>, long) + 357
 2   Electron Framework  0x000000013328dbcf (anonymous namespace)::ScreenshotManagerCapturer::OnRecurrentCaptureTimer() + 1343
 Stack trace:
 0   Electron Framework  0x000000012ade42f2 base::debug::CollectStackTrace(base::span<void const*, 18446744073709551615ul, void const**>) + 18
 1   Electron Framework  0x000000012add00e1 base::debug::StackTrace::StackTrace(unsigned long) + 225
 2   Electron Framework  0x000000012ade978a base::allocator::UnretainedDanglingRawPtrDetectedCrash(unsigned long) + 90
 3   Electron Framework  0x000000012ae437f7 base::internal::RawPtrBackupRefImpl<true>::ReportIfDanglingInternal(unsigned long) + 391

(cherry picked from commit f8d0412)

---------

Co-authored-by: Charles Kerr <charles@charleskerr.com>
Co-authored-by: Niklas Wenzel <dev@nikwen.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-backport semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants