On Tue, 4 Aug 2026 00:04:36 GMT, Alexander Matveev <[email protected]> wrote:

>> @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()`.
>
> I think we should fix it correctly. Original implementation was not right to 
> unmap memory on GstBuffer which might be released already.
> 
> My suggestion is to add functions below in CSample:
> `HRESULT SetGstBuffer(GstBuffer *pGstBuffer);`
> `GstBuffer *TakeGstBuffer();`
> 
> Add `m_bGstBufferMapped` flag as well to CSample.
> 
> Allocator will call `SetGstBuffer` and `SetGstBuffer` will map it. 
> `CSink::DoRenderSampleInternal()` will call `TakeGstBuffer` and this function 
> will unmap it and set `m_pGstBuffer` to NULL. Call `TakeGstBuffer` in 
> `CAllocator::ReleaseBuffer` to release it in case if it was not delivered. 
> `m_pGstBuffer` and `m_GstMapInfo` should be private.

I've made the requested changes, and updated the test results.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/2234#discussion_r3713922419

Reply via email to