Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Any Python objects that are used by callbacks should have their reference count incremented #14

Open
Keithcat1 opened this issue Mar 8, 2022 · 0 comments

Comments

@Keithcat1
Copy link

Keithcat1 commented Mar 8, 2022

On one hand, if I create a BASS channel sync or any object that our C function callback will expect to have access to, set it on a channel and then discard it, it's possible for the Python GC to garbage collect it. Then our callback we passed to BASS comes along, tries to get a reference to that sync object, but oops it's not there anymore. Being able to set syncs on a channel and then just forget them would be nice.
On the other hand, implementing this is kind of a pain. You can use the following Cython-defined macros to increment and decrement the reference count of an object.

from cpython.ref cimport PyObject, Py_DECREF, Py_INCREF

Increment the reference just before converting the PyObject* to void* in order to pass it as a userdata parameter to BASS, then decrement the reference count once we know BASS is done with it or the object will never be freed which causes a memory leak. Unfortunately BASS doesn't provide any way of knowing when a sync's userdata can be freed, so channels have to keep a list of the syncs applied to them or something and Py_DECREF them all along with any other objects owned by the channel when the channel is freed. The best way of knowing when a channel is freed that I know of is setting a BASS_SYNC_FREE on the channel, which can't be used on HSAMPLE channels. BASS_SampleGetChannel supports the BASS_SAMCHAN_STREAM flag which returns a HSTREAM which is the same as normal but it's a stream, supports more BASS features and has slightly lower performance compared to not using BASS_SAMCHAN_STREAM, so we could just make using BASS_SAMCHAN_STREAM the default I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant