Netflix / vmaf

Perceptual video quality assessment based on multi-method fusion.
Other
4.34k stars 735 forks source link

Fix frame double-free #1368

Closed xxie24 closed 3 weeks ago

xxie24 commented 4 weeks ago

Decrement refcnt and load the refcnt should be in 1 instruction in order to prevent data race in a multithreading environment.

nilfm99 commented 4 weeks ago

Hi, thanks for the contribution. I believe a race condition is still present with the fix, for example:

initially refcount is 1
thread A does fetch_decrement, brings it to 0
thread B does fetch_increment, brings it to 1
thread C does fetch_decrement, brings it back to 0
thread A proceeds to release it
thread C proceeds to release it

so I think we'll need locks to completely safeguard this, but I would agree that the race condition in the current code is way more likely to happen, so it may be good to do this in the interim.

nilfm99 commented 3 weeks ago

I'll merge this for now, as mentioned above I think there is still potential for data races but this is better than status quo. We'll revisit this later.