microsoft / microsoft/WindowsAppSDK

Windows.Data.Pdf.PdfDocument's destructor re-enters the message loop (C++/WinRT)

Open
#3,506 3 comments 0 reactions 0 assignees View on GitHub
area-External bug
Dominant language
C++
Stars
4.7k
Forks
471
Avg merge
3d 13h
Merged PRs (30d)
28

Description

### Describe the bug

This makes:

```
struct Foo : public Bar {
PdfDocument pdf;
};
```

... unsafe, if `Foo` is destructed from the main event loop;

this leads to:

1. Entering message loop
2. deleting Foo for some application-specific reason
3. Entering `~Foo`
4. Entering `~PdfDocument`
5. ... re-entering the message loop while we're still in `~Foo`, so:
- the pointer (raw/unique/shared/whatever) hasn't yet been cleared because deletion is in progress
- the object now has an invalid vtable
- depending on the application logic, this may lead to a double-free

### Steps to reproduce the bug

- create struct like above in event loop, with make_unique
- delete in event loop if set
- double-free

### Expected behavior

destructors should not enter message loop

### Screenshots

_No response_

### NuGet package version

Windows App SDK 1.2.4: 1.2.230217.4

### Packaging type

Unpackaged

### Windows version

Windows 10 version 22H2 (19045, 2022 Update)

### IDE

Other

### Additional context

Workaround:

```C++
PDFFilePageSource::~PDFFilePageSource() {
if (mPDFDocument) {
// Windows.Data.Pdf.PdfDocument's destructor re-enters the message loop -
// which means we can do:
// 1. Enter message loop
// 2. Enter ~PDFFilePageSource()
// 3. Enter ~PdfDocument()
// 4. Re-enter the message loop while `this` is partially destructed, and
// has an invalid vtable
// 5. make pure virtual calls, double-free, or other badness

// Work around this by rescheduling the deletion until 'later'
[](auto deleteLater) -> winrt::fire_and_forget {
// wil::resume_foreground guarantees that we will be rescheduled, even
// though we're not changing threads.
co_await wil::resume_foreground(winrt::Microsoft::UI::Dispatching::
DispatcherQueue::GetForCurrentThread());
}(std::move(mPDFDocument));
}
}
```

Contributor guide

No contributing guide indexed for this repository

Research direction

The report names no Windows App SDK source file or test. Start by reproducing the make_unique/delete case in the event loop on Windows App SDK 1.2.4, then inspect whether the behavior is controlled by this repository or by Windows.Data.Pdf. Done means establishing an actionable fix or documenting that the supplied deferred-deletion workaround is required.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
desktop
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.