Skip to content

workaround PySide QByteArray leak on conda builds#3268

Merged
j9ac9k merged 1 commit intopyqtgraph:masterfrom
pijyoi:workaround-qbytearray-leak
Mar 2, 2025
Merged

workaround PySide QByteArray leak on conda builds#3268
j9ac9k merged 1 commit intopyqtgraph:masterfrom
pijyoi:workaround-qbytearray-leak

Conversation

@pijyoi
Copy link
Copy Markdown
Contributor

@pijyoi pijyoi commented Feb 28, 2025

Addresses #3265 at the expense of penalizing the official Qt build of PySide6.

The leak only gets triggered for connect types "pairs", "array".
"finite" is also affected if there are a great many non-finite elements.

Note that 2 extra memory writes are made with this workaround.

  1. bytearray always initializes its memory to 0
  2. when the bytearray is converted into a QByteArray.
    (this second memory access could have been avoided if QByteArray.fromRawData was working. But alas, the binding for it is broken)

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Feb 28, 2025

Since the "fix" has been merged into the conda-forge feedstock, do we want to have the workaround enabled only for specific versions of pyside bindings; or is there minimal downside to the workaround that that's not needed?

@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Feb 28, 2025

I did a check, the issue is also present in conda-forge's build of PySide2 5.15.15.
The issue is not present in Qt's official build PySide2 5.15.2.1.

refcnt leak introduced:
pyside/pyside2-setup@821480b

the following comment says that Qt defaults to limited-api builds (and therefore their builds aren't affected by the leak introduced):
pyside/pyside2-setup@032cb10

I couldn't find where pyside2-feedstock was configuring for non-limited-api build though.

Given that conda-forge has patched PySide2 to build for Python 3.11, and seem willing (conda-forge/pyside2-feedstock#246) to push it to Python 3.12, what is pyqtgraph's policy regarding support of PySide2 going to be?

do we want to have the workaround enabled only for specific versions of pyside bindings;

For PySide6, we can don't apply the workaround. conda-forge PySide6 6.7.3 through PySide6 6.8.2 build 0 will have the leak.

For PySide2, maybe we can apply the workaround? If PySide2 is a non-preferred binding for pyqtgraph, I guess penalizing it is not really an issue? i.e. better to be less efficient than having to worry about whether a leak is present for non-official builds.

@pijyoi pijyoi force-pushed the workaround-qbytearray-leak branch from e4d7745 to 664ab25 Compare February 28, 2025 23:12
@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Mar 1, 2025

Other places where the leak occurs:

  • Ubuntu 22.04 / Python 3.10.12 / PySide2 5.15.2
  • Ubuntu 24.04 / Python 3.12.3 / PySide2 5.15.13

@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Mar 1, 2025

For reference, pyqtgraph first started taking a memory view to QByteArray in this commit. (2021 August)
287ec9f

PySide2 5.15.2 had already been released for some time.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 2, 2025

Thanks @pijyoi !

@j9ac9k j9ac9k merged commit 4b4d7a6 into pyqtgraph:master Mar 2, 2025
36 checks passed
@pijyoi pijyoi deleted the workaround-qbytearray-leak branch March 2, 2025 07:19
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

Successfully merging this pull request may close these issues.

2 participants