Incorrect handling of reference counting for COM #97338

New Issue

Robert Guetzkow · 2022-04-15T01:06:01+02:00

Robert Guetzkow commented

2022-04-15 01:06:01 +02:00

Blender Version
Broken: 8825250f5a

Short description of error
The issues affect the function delete_soft (source\blender\blenlib\intern\fileops.c)

The reference count for IShellItem *pSI; needs to be decremented. It currently is not, thus creating a memory leak. This issue was originally discovered by Rolf Kalbermatter
The reference count for IFileOperation *pfo; may only be decremented if CoCreateInstance is successful. It is currently done in both successful and failed calls. In the latter case, this would result in dereferencing a NULL pointer.

**Blender Version** Broken: 8825250f5a **Short description of error** The issues affect the function `delete_soft` (`source\blender\blenlib\intern\fileops.c`) - The reference count for `IShellItem *pSI;` needs to be decremented. It currently is not, thus creating a memory leak. This issue was originally discovered by [Rolf Kalbermatter ](https://github.com/RolfKal) - The reference count for `IFileOperation *pfo;` may only be decremented if `CoCreateInstance` is successful. It is currently done in both successful and failed calls. In the latter case, this would result in dereferencing a `NULL` pointer.

Robert Guetzkow commented

2022-04-15 01:06:01 +02:00

Added subscriber: @rjg

Robert Guetzkow self-assigned this 2022-04-15 01:06:25 +02:00

Robert Guetzkow commented

2022-04-15 01:14:20 +02:00

Besides the issue covered in this ticket, the (un)initialization of COM should be implemented in one place. Instead, it is currently being initialized in:

WASAPIDevice::WASAPIDevice (extern\audaspace\plugins\wasapi\WASAPIDevice.cpp) with COINIT_MULTITHREADED
delete_soft (source\blender\blenlib\intern\fileops.c) with COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE
BLI_file_alias_target (source\blender\blenlib\intern\storage.c) with COINIT_MULTITHREADED

The delete_soft and BLI_file_alias_target also call CoUninitialize, but WASAPIDevice::WASAPIDevice does not. This issue would need a separate design task to discuss the proper implementation of the COM initialization per thread.

Besides the issue covered in this ticket, the (un)initialization of COM should be implemented in one place. Instead, it is currently being initialized in: - `WASAPIDevice::WASAPIDevice` (`extern\audaspace\plugins\wasapi\WASAPIDevice.cpp`) with `COINIT_MULTITHREADED` - `delete_soft` (`source\blender\blenlib\intern\fileops.c`) with `COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE` - `BLI_file_alias_target` (`source\blender\blenlib\intern\storage.c`) with `COINIT_MULTITHREADED` The `delete_soft` and `BLI_file_alias_target` also call `CoUninitialize`, but `WASAPIDevice::WASAPIDevice` does not. This issue would need a separate design task to discuss the proper implementation of the COM initialization per thread.

Pratik Borhade commented

2022-04-15 05:57:36 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'