From e522e8d262c0a1b692bb23f43549fb4090fcdebb Mon Sep 17 00:00:00 2001
From: moon2 <moon2protonmail@protonmail.com>
Date: Tue, 17 Dec 2024 13:46:46 +0100
Subject: [PATCH] bl00mbox: fix weakref behavior
---
components/bl00mbox/bl00mbox_audio.c | 12 +++-
components/bl00mbox/include/bl00mbox_audio.h | 1 +
.../bl00mbox/micropython/bl00mbox/_user.py | 28 +++++---
.../bl00mbox/micropython/mp_sys_bl00mbox.c | 65 ++++++++++---------
4 files changed, 66 insertions(+), 40 deletions(-)
diff --git a/components/bl00mbox/bl00mbox_audio.c b/components/bl00mbox/bl00mbox_audio.c
index 9137ffed39..d5d79ce704 100644
--- a/components/bl00mbox/bl00mbox_audio.c
+++ b/components/bl00mbox/bl00mbox_audio.c
@@ -175,10 +175,20 @@ void bl00mbox_channel_destroy(bl00mbox_channel_t * chan){
// remove from parent
if(* (chan->parent_self_ref) != chan){
- bl00mbox_log_error("channel: parent_self_ref improper: channel %p, ref %p", chan, * (chan->parent_self_ref));
+ bl00mbox_log_error("channel: parent_self_ref improper: channel %p, ref %p",
+ chan, * (chan->parent_self_ref));
}
* (chan->parent_self_ref) = NULL;
+ // remove from weak parent
+ if(chan->weak_parent_self_ref){
+ if(* (chan->weak_parent_self_ref) != chan){
+ bl00mbox_log_error("channel: weak_parent_self_ref improper: channel %p, ref %p",
+ chan, * (chan->weak_parent_self_ref));
+ }
+ * (chan->weak_parent_self_ref) = NULL;
+ }
+
#ifdef BL00MBOX_DEBUG
if(chan->connections.len) bl00mbox_log_error("connections nonempty");
if(chan->roots.len) bl00mbox_log_error("roots nonempty");
diff --git a/components/bl00mbox/include/bl00mbox_audio.h b/components/bl00mbox/include/bl00mbox_audio.h
index b23af0aaa9..41552c44a1 100644
--- a/components/bl00mbox/include/bl00mbox_audio.h
+++ b/components/bl00mbox/include/bl00mbox_audio.h
@@ -89,6 +89,7 @@ typedef struct _bl00mbox_channel_t{
void * parent;
struct _bl00mbox_channel_t ** parent_self_ref;
+ struct _bl00mbox_channel_t ** weak_parent_self_ref;
} bl00mbox_channel_t;
void bl00mbox_audio_init();
diff --git a/components/bl00mbox/micropython/bl00mbox/_user.py b/components/bl00mbox/micropython/bl00mbox/_user.py
index 893a69048d..2f99bca564 100644
--- a/components/bl00mbox/micropython/bl00mbox/_user.py
+++ b/components/bl00mbox/micropython/bl00mbox/_user.py
@@ -446,9 +446,9 @@ class Channel:
def __init__(self, name="repl"):
# never ever hand out the deep core to somebody else to make sure channel gets gc'd right
self._deep_core = sys_bl00mbox.ChannelCore(name)
- self._core = self._deep_core.make_fake()
+ self._core = self._deep_core.get_weak()
# also don't hand out self, use this one instead
- self._fake_channel = FakeChannel(self._core)
+ self._weak_channel = WeakChannel(self._core)
if _channel_init_callback is not None:
_channel_init_callback(self)
self._channel_plugin = bl00mbox._plugins._get_plugin(self._core.channel_plugin)
@@ -484,7 +484,7 @@ class Channel:
def _new_plugin(self, thing, *args, **kwargs):
if isinstance(thing, bl00mbox._plugins._PluginDescriptor):
return bl00mbox._plugins._make_plugin(
- self._fake_channel, thing.plugin_id, *args, **kwargs
+ self._weak_channel, thing.plugin_id, *args, **kwargs
)
else:
raise TypeError("not a plugin")
@@ -493,7 +493,7 @@ class Channel:
self.free = False
if type(thing) == type:
if issubclass(thing, bl00mbox.patches._Patch):
- return thing(self._fake_channel, *args, **kwargs)
+ return thing(self._weak_channel, *args, **kwargs)
elif isinstance(thing, bl00mbox._plugins._PluginDescriptor) or (
type(thing) == int
):
@@ -556,14 +556,20 @@ class Channel:
raise Bl00mboxError("can't connect this")
-class FakeChannel(Channel):
+class WeakChannel(Channel):
def __init__(self, core):
- self._core = core
+ self._core = core.get_weak()
self._channel_plugin = bl00mbox._plugins._get_plugin(self._core.channel_plugin)
- self._fake_channel = self
+ self._weak_channel = self
+
+class SysChannel(Channel):
+ def __init__(self, core):
+ self._deep_core = core
+ self._core = core.get_weak()
+ self._channel_plugin = bl00mbox._plugins._get_plugin(self._core.channel_plugin)
+ self._weak_channel = WeakChannel(self._core)
-class SysChannel(FakeChannel):
@property
def sys_gain_dB(self):
ret = self._core.sys_gain
@@ -615,10 +621,12 @@ class Sys:
core.foreground = False
@staticmethod
- def collect_channels(active):
+ def collect_channels(active, weak_ref=False):
cores = sys_bl00mbox.collect_channels(active)
for core in cores:
- yield SysChannel(core.make_fake())
+ if weak_ref:
+ core = core.get_weak()
+ yield SysChannel(core)
@staticmethod
def collect_active_callbacks():
diff --git a/components/bl00mbox/micropython/mp_sys_bl00mbox.c b/components/bl00mbox/micropython/mp_sys_bl00mbox.c
index e1b8a5450f..441d02cb95 100644
--- a/components/bl00mbox/micropython/mp_sys_bl00mbox.c
+++ b/components/bl00mbox/micropython/mp_sys_bl00mbox.c
@@ -18,16 +18,20 @@ MP_DEFINE_EXCEPTION(ReferenceError, Exception)
typedef struct _channel_core_obj_t {
mp_obj_base_t base;
- // a fake reference never deletes the channel
- bool fake;
- // points to chan of original. dereferenced value might be NULL if
- // channel has been manually deleted, use channel_core_verify()
- bl00mbox_channel_t **chan_ref;
// will be NULLed on original if channel has been manually deleted
+ // both for weak and regular
bl00mbox_channel_t *chan;
+
+ // pointer to object that weakly references original channel object
+ // there is no more than one weak channel reference obj per channel
+ mp_obj_t weak;
+
+ // things we don't want garbage collected:
+ // - channel plugin
mp_obj_t channel_plugin;
- // list of all sources connect to the mixer to avoid gc
+ // - list of all sources connect to the mixer
mp_obj_t sources_mx;
+
#ifdef BL00MBOX_CALLBACKS_FUNSAFE
// the channel may be become reachable after having been unreachable.
// if the callback was collected during this phase it results in a
@@ -74,7 +78,6 @@ STATIC mp_obj_t signal_core_make_new(const mp_obj_type_t *type, size_t n_args,
size_t n_kw, const mp_obj_t *args);
static inline void channel_core_verify(channel_core_obj_t *channel_core) {
- channel_core->chan = *channel_core->chan_ref;
if (!channel_core->chan)
mp_raise_msg(&mp_type_ReferenceError,
MP_ERROR_TEXT("channel was deleted"));
@@ -252,10 +255,27 @@ static mp_obj_t create_channel_plugin(bl00mbox_channel_t *chan) {
return MP_OBJ_FROM_PTR(self);
}
+static mp_obj_t create_weak_channel_core(channel_core_obj_t *self) {
+ channel_core_obj_t *weak = m_new_obj_with_finaliser(channel_core_obj_t);
+ weak->base.type = &channel_core_type;
+ weak->weak = MP_OBJ_FROM_PTR(weak);
+ weak->chan = self->chan;
+ weak->chan->weak_parent_self_ref = &weak->chan;
+
+ // only the real channel should prevent these from being gc'd
+ weak->sources_mx = MP_OBJ_NULL;
+ weak->channel_plugin = MP_OBJ_NULL;
+#ifdef BL00MBOX_CALLBACKS_FUNSAFE
+ weak->callback = MP_OBJ_NULL;
+#endif
+ return MP_OBJ_FROM_PTR(weak);
+}
+
STATIC mp_obj_t channel_core_make_new(const mp_obj_type_t *type, size_t n_args,
size_t n_kw, const mp_obj_t *args) {
mp_arg_check_num(n_args, n_kw, 1, 1, false);
channel_core_obj_t *self = m_new_obj_with_finaliser(channel_core_obj_t);
+ self->chan = NULL; // must initialize if obj gets gc'd due to exception
self->base.type = &channel_core_type;
self->sources_mx = mp_obj_new_list(0, NULL);
@@ -269,8 +289,7 @@ STATIC mp_obj_t channel_core_make_new(const mp_obj_type_t *type, size_t n_args,
chan->parent = self;
self->chan = chan;
- self->chan_ref = &self->chan;
- self->fake = false;
+ self->weak = create_weak_channel_core(self);
self->channel_plugin = create_channel_plugin(chan);
#ifdef BL00MBOX_CALLBACKS_FUNSAFE
self->callback = mp_const_none;
@@ -279,29 +298,17 @@ STATIC mp_obj_t channel_core_make_new(const mp_obj_type_t *type, size_t n_args,
return MP_OBJ_FROM_PTR(self);
}
-STATIC mp_obj_t mp_channel_core_make_fake(mp_obj_t self_in) {
- channel_core_obj_t *fake = m_new_obj_with_finaliser(channel_core_obj_t);
- fake->base.type = &channel_core_type;
-
+STATIC mp_obj_t mp_channel_core_get_weak(mp_obj_t self_in) {
channel_core_obj_t *self = MP_OBJ_TO_PTR(self_in);
channel_core_verify(self);
-
- fake->sources_mx = mp_const_none;
- fake->chan = self->chan;
- fake->chan_ref = self->chan_ref;
- fake->fake = true;
- fake->channel_plugin = self->channel_plugin;
-#ifdef BL00MBOX_CALLBACKS_FUNSAFE
- fake->callback = MP_OBJ_NULL;
-#endif
- return MP_OBJ_FROM_PTR(fake);
+ return self->weak;
}
-MP_DEFINE_CONST_FUN_OBJ_1(mp_channel_core_make_fake_obj,
- mp_channel_core_make_fake);
+MP_DEFINE_CONST_FUN_OBJ_1(mp_channel_core_get_weak_obj,
+ mp_channel_core_get_weak);
mp_obj_t mp_channel_core_del(mp_obj_t self_in) {
channel_core_obj_t *self = MP_OBJ_TO_PTR(self_in);
- if (self->chan && !self->fake) {
+ if (self->chan && (self->weak != self_in)) {
bl00mbox_log_info("destroyed channel %s", self->chan->name);
bl00mbox_channel_destroy(self->chan);
}
@@ -392,7 +399,7 @@ STATIC void channel_core_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) {
dest[0] = mp_obj_new_str(ret, strlen(ret));
free(ret);
} else if (attr == MP_QSTR_channel_plugin) {
- dest[0] = self->channel_plugin;
+ dest[0] = ((channel_core_obj_t *)chan->parent)->channel_plugin;
} else if (attr == MP_QSTR_callback) {
#ifdef BL00MBOX_CALLBACKS_FUNSAFE
dest[0] = ((channel_core_obj_t *)chan->parent)->callback;
@@ -419,8 +426,8 @@ STATIC const mp_rom_map_elem_t channel_core_locals_dict_table[] = {
{ MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&mp_channel_core_del_obj) },
{ MP_ROM_QSTR(MP_QSTR_delete), MP_ROM_PTR(&mp_channel_core_delete_obj) },
{ MP_ROM_QSTR(MP_QSTR_clear), MP_ROM_PTR(&mp_channel_core_clear_obj) },
- { MP_ROM_QSTR(MP_QSTR_make_fake),
- MP_ROM_PTR(&mp_channel_core_make_fake_obj) },
+ { MP_ROM_QSTR(MP_QSTR_get_weak),
+ MP_ROM_PTR(&mp_channel_core_get_weak_obj) },
{ MP_ROM_QSTR(MP_QSTR_get_connected_mx),
MP_ROM_PTR(&mp_channel_core_get_connected_mx_obj) },
{ MP_ROM_QSTR(MP_QSTR_get_plugins),
--
GitLab