ckerr

#54513: fix: clear DownloadItem backlink before native removal

Merged
Created: Sep 28, 2026, 11:10:03 AM
Merged: Sep 28, 2026, 12:35:36 PM
2 comments
Target: main

Summary

The DownloadItem destructor unregisters its observer before calling the native item's Remove(), so native destruction no longer clears download_item_ through OnDownloadDestroyed().

Clear the member before Remove() synchronously deletes the native item, keeping a stack-local pointer for the call. This avoids a dangling raw_ptr during wrapper teardown while preserving observer removal and download cancellation behavior.

With raw_ptr checks enabled, the focused test reported the following diagnostic (unrelated stack frames omitted):

[DanglingPtr](1/3) A raw_ptr/raw_ref is dangling.

[DanglingPtr](2/3) First, the memory was freed at:

Stack trace:
...
#4 0x639c60ee44c6 download::DownloadItemImpl::~DownloadItemImpl() [../../components/download/internal/common/download_item_impl.cc:538:39]
#5 0x639c603400b6 std::__Cr::unordered_map<>::erase[abi:sqn240000]() [../../third_party/libc++/src/include/__memory/unique_ptr.h:74:5]
#6 0x639c6033ff9d content::DownloadManagerImpl::DownloadRemoved() [../../content/browser/download/download_manager_impl.cc:1169:14]
#7 0x639c60ee6ddf download::DownloadItemImpl::Remove() [../../components/download/internal/common/download_item_impl.cc:760:14]
#8 0x639c5a50a3ad electron::api::DownloadItem::~DownloadItem() [../../electron/shell/browser/api/electron_api_download_item.cc:96:21]
...

[DanglingPtr](3/3) Later, the dangling raw_ptr was released at:

Stack trace:
...
#2 0x639c6247095d base::allocator::(anonymous namespace)::DanglingRawPtrReleased<>() [../../base/allocator/partition_alloc_support.cc:630:21]
#3 0x639c624b97c2 base::internal::RawPtrBackupRefImpl<>::ReleaseInternal() [../../base/allocator/partition_allocator/src/partition_alloc/in_slot_metadata.h:240:7]
#4 0x639c5a50a48d electron::api::DownloadItem::~DownloadItem() [../../base/allocator/partition_allocator/src/partition_alloc/pointers/raw_ptr_backup_ref_impl.h:194:7]
...

Validation

  • Confirmed the dangling-pointer error before applying the fix, then rebuilt Electron and confirmed the same focused test passed without that error after the fix, with raw_ptr checks enabled in both runs: should be released after the download completes in spec/cpp-heap.spec.ts.
  • All three DownloadItem heap tests passed: surviving GC during an in-progress download, release after completion, and no leak across multiple downloads.
  • git diff --check passed.
  • Targeted C++ lint and clang-format checks passed for the changed file.

Notes: Fixed a potential crash when cleaning up download items.

Backports

45-x-y
In-flight
PR Number
#54517
Waiting to be merged

Semver Impact

Major
Breaking changes
Minor
New features
Patch
Bug fixes
None
Docs, tests, etc.

Semantic Versioning helps users understand the impact of updates:

  • Major (X.y.z): Breaking changes that may require code modifications
  • Minor (x.Y.z): New features that maintain backward compatibility
  • Patch (x.y.Z): Bug fixes that don't change the API
  • None: Changes that don't affect using facing parts of Electron