Multiple instances of potential garbage collection of live widgets

[edwardwkrohne]

If I instantiate a widget like

get_state, set_state = mo.state(False)
wgt = mo.ui.checkbox(on_change=set_state)
wgt

get_state()

then all is well: the second cell updates as I click. If, on the other hand, if I instantiate it like

get_state, set_state = mo.state(False)
wgt = mo.ui.checkbox(on_change=set_state).style()
wgt

get_state()

then set_state is never called and the second cell is frozen as False.

The issue is that style returns

    return Html(h.div(children=as_dom_node(item).text, style=style_str))

This treats item as nothing more than the sum of its html string as_dom_node(item).text, wraps that html string into the h.div, and returns the composite Html object, dropping item on the floor. In typical marimo usage, this isn't an issue because item is kept alive as a global or some child of some global. But if the user creates item in such a way that no strong reference to it exists, for example because it's part of a dynamic tree of styled widgets, item is vulnerable to garbage collection and the Javascript representation of the widget loses connection with its Python half.

This is a bug in a variety of places; simply patching style is insufficient. I haven't been through them all, but according to Claude Opus,

• mo.style/.style()
• mo.callout/.callout()
• mo.accordion
• mo.carousel
• mo.ui.tabs
• mo.sidebar
• mo.stat(slot=...)

are all prone to dropping references and allowing garbage collection.

There is already a solution in the code, though, for vstack and hstack: the flexboxes inherit from flex._FlexContainerHtml which explicitly keeps alive its children by storing references to them, and hypertext._BlockWrapped does something similar.

I propose to add a class called ContainerHtml which explicitly keeps alive references to its children. It's essentially just _BlockWrapped with a little flexibility built in:

class ContainerHtml(Html):
    def __init__(self, children: Sequence[Html]) -> None:
        self._children: list[Html] = list(children)
        super().__init__(self._build_text())

    def _build_text(self) -> str:
        raise NotImplementedError

    @property
    def text(self) -> str:  # type: ignore[override]
        """Re-render children live on every access."""
        return self._build_text()

Functions like style and accordion would return a class like StyledHtml and AccordionHtml, derived from ContainerHtml, with their own implementations of __init__ and _build_text. Existing, similar classes like _BlockWrapped would be renamed/reparented for consistency. Uses of as_dom_node/as_html to create containers would now all involve these container classes, so anyone who goes to make something new by reading old code would be more likely to adopt the new pattern instead of dropping the reference.

Potentially some refactor of as_dom_node/as_html to make them impossible to use without one of these classes would be warranted, but that would be more complicated and take a lot more thought.

I have a partial PR where I've applied the fix to style and accordion and it works great -- code similar to both snippets above functions correctly. But this represents a refactor of central widget objects, including the stacks, and so as noted in CONTRIBUTING.md, I'm making a proposal and asking for feedback before I proceed further on this.

Will you submit a PR?

• Yes

Environment

Details

{
  "marimo": "0.23.9",
  "editable": true,
  "location": "/home/krohne/marimo-dev/marimo",
  "OS": "Linux",
  "OS Version": "6.14.0-37-generic",
  "Processor": "x86_64",
  "Python Version": "3.13.12",
  "Locale": "--",
  "Binaries": {
    "Browser": "148.0.7778.215",
    "Node": "--",
    "uv": "0.11.4 (x86_64-unknown-linux-gnu)"
  },
  "Dependencies": {
    "click": "8.2.1",
    "docutils": "0.23",
    "itsdangerous": "2.2.0",
    "jedi": "0.19.2",
    "markdown": "3.10.2",
    "narwhals": "2.22.0",
    "packaging": "26.2",
    "psutil": "7.2.2",
    "pygments": "2.20.0",
    "pymdown-extensions": "10.21.3",
    "pyyaml": "6.0.3",
    "starlette": "1.2.1",
    "tomlkit": "0.15.0",
    "typing-extensions": "4.15.0",
    "uvicorn": "0.49.0",
    "websockets": "16.0"
  },
  "Optional Dependencies": {
    "altair": "6.1.0",
    "anywidget": "0.9.21",
    "duckdb": "1.5.3",
    "loro": "1.10.3",
    "mcp": "1.27.2",
    "nbformat": "5.10.4",
    "openai": "2.41.0",
    "pandas": "3.0.3",
    "polars": "1.41.2",
    "pyarrow": "24.0.0",
    "pytest": "9.0.3",
    "ruff": "0.15.16",
    "sqlglot": "30.9.0"
  },
  "Experimental Flags": {
    "multi_column": true,
    "cache_panel": true,
    "isolate_apps": true
  }
}

Code to reproduce

No response

[manzt]

In the other cases the container/output object already holds the reference (_BlockWrapped, _FlexContainerHtml), so generalizing that into a shared ContainerHtml seems reasonable. You mentioned you had a branch with some changes. Would you be interesting in contributing a PR?

[edwardwkrohne]

I'd love to. I'll let you know when I've got it together.

[edwardwkrohne]

One question though: do you have a preference on where that class would live?

[edwardwkrohne]

marimo/_plugins/stateless/stat.py is a bit different. It seems specifically intended to display non-interactive HTML objects in its slot field. I was going to propose to add a fail-fast check that <marimo-ui-element didn't appear in its input since that's a reasonable heuristic that an object is static, but I found try_convert_to_html which eats any problems converting slot to html and downgrades the exception to a log error message. That's doing the opposite of what I was going to suggest: it's making unexpected cases quieter, not louder.

Why are you eating those exceptions?

Anyway, the others I mentioned are done and I'm putting together the PR.