On Fri, 31 Jul 2026 23:01:49 GMT, Kevin Rushforth <[email protected]> wrote:
>> This is the fix for "JDK-8389515: Memory leak in dshowwrapper - decoded >> buffers delivered by CSink are never unmapped" >> >> The fix adds a second field `m_pMappedGstBuffer` that tracks mapping >> ownership independently of the `m_pGstBuffer` delivery ownership that >> `CSink` steals, so the unmap always targets the buffer that was actually >> mapped, and it is done exactly once. The fix conforms to the buffer life >> cycle -- `CAllocator::GetBuffer()` - `CAllocator::ReleaseBuffer()`. >> >> The reproducer log with the fix applied: >> >> + d:/work/jdk/jdk25/bin/java.exe @jfx/build/run.args >> MemoryLeakReproducer.java demo.mp4 >> Java: 25 (OpenJDK 64-Bit Server VM) >> JavaFX: 28-internal (runtime 28-internal+0-2026-07-31-112708) >> 1015.8 MiB >> 1035.6 MiB >> 1131.3 MiB >> 1132.9 MiB >> 1137.8 MiB >> 1273.0 MiB >> >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > modules/javafx.media/src/main/native/gstreamer/plugins/dshowwrapper/Allocator.cpp > line 92: > >> 90: if (pSample->m_pMappedGstBuffer != NULL) >> 91: { >> 92: gst_buffer_unmap(pSample->m_pMappedGstBuffer, >> &pSample->m_GstMapInfo); > > `m_pMappedGstBuffer` is a copy of the pointer that was used to initialize > `m_pGstBuffer`, but there is nothing that will ensure ownership of the > buffer. If there is any code path where the buffer is freed prior to > `CAllocator::ReleaseBuffer` being called -- and it looks like there might be > -- this could lead to a use after free. > > I'd like @sashamatveev to take a closer look. @kevinrushforth > If there is any code path where the buffer is freed prior to > CAllocator::ReleaseBuffer ... this could lead to a use after free. I would only like to point out that the proposed change is intended to _upmap_ the buffer properly, and repeats the original behavior that has been disrupted after disabling the macro. OTOH we could move unmapping closer to ownership change in `CSink::DoRenderSampleInternal()`. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2234#discussion_r3695934786
