mirror of
https://github.com/electron/electron.git
synced 2026-05-02 03:00:22 -04:00
fix: remove early capturer_.reset() that causes nullptr deref on next refresh (#51329)
fix: remove early capturer_.reset() that causes null deref on next refresh Another followup todad4ab658a: remove the `capturer_.reset()` that `desktop_media_list.patch` was adding `Worker::RefreshNextThumbnail()`. Since we switched from the one-shot Update() model to the continuous StartUpdating() model, resetting `capturer_` isn't necessary and is now dangerous: ScheduleNextRefresh() posts a delayed Worker::Refresh() that dereferences capturer_, causing a nullptr crash. Under CI load, the NativeDesktopMediaList can survive long enough for the next 1-second refresh cycle to fire before FinalizeList() destroys it. The crash can manifest as either a SIGSEGV or a DCHECK(can_refresh()) failure, which is extra fun becausedad4ab658awas fixing a similar DCHECK crash in the first place. Sample crash: ``` [6690:0426/173732.876803:FATAL:chrome/browser/media/webrtc/native_desktop_media_list.cc:934] DCHECK failed: can_refresh().0x00000001337aa7f3 NativeDesktopMediaList::RefreshForVizFrameSinkWindows(...) + 131 ```
This commit is contained in:
@@ -54,7 +54,7 @@ index de56c9b94f92e9abf69b1d4894e5d386cad6d3cd..f8955ef7cc43b1854b29841ed65260a1
|
||||
const Source& GetSource(int index) const override;
|
||||
DesktopMediaList::Type GetMediaListType() const override;
|
||||
diff --git a/chrome/browser/media/webrtc/native_desktop_media_list.cc b/chrome/browser/media/webrtc/native_desktop_media_list.cc
|
||||
index 2b745dbb254c714756a953ac0a32c1430af2c91d..9a8ebb4edfb92d9fe28ae4b87463a68547ea1ab3 100644
|
||||
index 2b745dbb254c714756a953ac0a32c1430af2c91d..eb148923593b4651a1ac3c34c35b8f75beafa143 100644
|
||||
--- a/chrome/browser/media/webrtc/native_desktop_media_list.cc
|
||||
+++ b/chrome/browser/media/webrtc/native_desktop_media_list.cc
|
||||
@@ -216,9 +216,13 @@ content::DesktopMediaID::Id GetUpdatedWindowId(
|
||||
@@ -71,29 +71,7 @@ index 2b745dbb254c714756a953ac0a32c1430af2c91d..9a8ebb4edfb92d9fe28ae4b87463a685
|
||||
#endif
|
||||
|
||||
return window_id;
|
||||
@@ -321,7 +325,7 @@ class NativeDesktopMediaList::Worker
|
||||
base::WeakPtr<NativeDesktopMediaList> media_list_;
|
||||
|
||||
DesktopMediaID::Type source_type_;
|
||||
- const std::unique_ptr<ThumbnailCapturer> capturer_;
|
||||
+ std::unique_ptr<ThumbnailCapturer> capturer_;
|
||||
const ThumbnailCapturer::FrameDeliveryMethod frame_delivery_method_;
|
||||
const bool add_current_process_windows_;
|
||||
const bool auto_show_delegated_source_list_;
|
||||
@@ -603,6 +607,12 @@ void NativeDesktopMediaList::Worker::RefreshNextThumbnail() {
|
||||
FROM_HERE,
|
||||
base::BindOnce(&NativeDesktopMediaList::UpdateNativeThumbnailsFinished,
|
||||
media_list_));
|
||||
+
|
||||
+ // This call is necessary to release underlying OS screen capture mechanisms.
|
||||
+ // Skip if the source list is delegated, as the source list window will be active.
|
||||
+ if (!capturer_->GetDelegatedSourceListController()) {
|
||||
+ capturer_.reset();
|
||||
+ }
|
||||
}
|
||||
|
||||
void NativeDesktopMediaList::Worker::OnCaptureResult(
|
||||
@@ -1015,6 +1025,11 @@ void NativeDesktopMediaList::RefreshForVizFrameSinkWindows(
|
||||
@@ -1015,6 +1019,11 @@ void NativeDesktopMediaList::RefreshForVizFrameSinkWindows(
|
||||
FROM_HERE, base::BindOnce(&Worker::RefreshThumbnails,
|
||||
base::Unretained(worker_.get()),
|
||||
std::move(native_ids), thumbnail_size_));
|
||||
|
||||
@@ -13,10 +13,10 @@ This patch fixes the crash by ensuring COM is initialized on the
|
||||
capture thread by calling `init_com_with_mta(false)`.
|
||||
|
||||
diff --git a/chrome/browser/media/webrtc/native_desktop_media_list.cc b/chrome/browser/media/webrtc/native_desktop_media_list.cc
|
||||
index 9a8ebb4edfb92d9fe28ae4b87463a68547ea1ab3..13446d9849c54f1bfe515c3db4d69dd181ec6d39 100644
|
||||
index eb148923593b4651a1ac3c34c35b8f75beafa143..f023e27c28f7464dae6466c855eef5804a8c6cdb 100644
|
||||
--- a/chrome/browser/media/webrtc/native_desktop_media_list.cc
|
||||
+++ b/chrome/browser/media/webrtc/native_desktop_media_list.cc
|
||||
@@ -786,6 +786,13 @@ NativeDesktopMediaList::NativeDesktopMediaList(
|
||||
@@ -780,6 +780,13 @@ NativeDesktopMediaList::NativeDesktopMediaList(
|
||||
base::MessagePumpType thread_type = base::MessagePumpType::UI;
|
||||
#else
|
||||
base::MessagePumpType thread_type = base::MessagePumpType::DEFAULT;
|
||||
|
||||
Reference in New Issue
Block a user