FIX: Decrease figure refcount on close of a macosx figure #23059
Conversation
|
Is there a way to test this that you used? |
|
I was modifying: #19769 (comment) Which will send some count information out, which should show FigureManager in it before and fewer of them now. If you cherry-pick this commit over to v3.5.x and test you will see a major difference though and no more memory leak/FigureManager at all, but I still need to figure out how to fix the second Timer memory leak now... So, this fixes one of the leaks, but not both unfortunately. A step in the right direction... |
|
I just pushed a new commit: 0822fd0 that briefly runs the event loop before destroying the figure. This seems to clean up the memory leak. The idea was taken from: #23061 (comment), or at least I think that is what @tacaswell meant! I tried to use "flush_events", which didn't work, so maybe that should be modified (?) as well. However, if I tried to put the To test this, you should be able to run There may be another memory leak in FontProperties, as I'm seeing that hang around in the growing counts here, so I don't get that jagged line like others were upon garbage collection, even with Agg as the backend. |
|
Just putting some notes down here about the mac leaks I've found so far:
|
MacOSXでsavefigするとメモリリークする問題(※)への対応 # [Bug]: macosx timers don't fire if plt.show() hasn't been called # matplotlib/matplotlib#23061 # FIX: Decrease figure refcount on close of a macosx figure #23059 # matplotlib/matplotlib#23059 # # 解決策: 描画処理を別プロセスに隔離するように変更 # 参考にしたページ: https://qiita.com/Masahiro_T/items/037c118c9dd26450f8c8
|
Can you please rebase to confirm that all (likely unrelated) CI problems are gone? |
|
Popping to draft pending revision... |
0822fd0 to
364194e
Compare
|
OK, I've reworked this now to keep track of the timers on the canvas and then iterate through them on close to clear out any pending timers, let me know your thoughts on this approach instead of the event loop spinning. The problem is that we don't ever start the mainloop when just calling @dstansby pinging you as well to see if this fixes what you were seeing since you were looking into some of this recently: #19769 (comment) |
We need to take the refcount of the figuremanager down when we close a window. This was leaking figures before.
Running the macosx backend without calling show() would cause Timers to pile up and not fire because the event loop was not running. This leaked objects when closing/opening multiple figures. To fix this, we keep track of the timers on the canvas and remove all of the pending timers when destroying the figure to clear all back-references.
364194e to
fe6366d
Compare
|
@tacaswell since you're currently blocking, I'll ping for another look at the event loop work now that I've updated it with a set of timers and get your thoughts on that approach. |
PR Summary
We need to take the refcount of the figuremanager down when we close a window. (apparently dealloc on the Window isn't being called?) We could call this within the FigureManager_dealloc method, or just inline it here because close gets called during the FigureManager_dealloc already, so I decided this simplifies the code.
This was causing leaking figures before, per: #19769
and this PR does fix the issue on v3.5.x, but unfortunately there is a new leak that I introduced in #21790 :( There are no leaks when we
plt.show()figures and fire up the GUI mainloop. However, when we are callingsavefig()without ever showing we are now creating singleshot Timers in the app, but the callback never gets triggered due to the event loop not being active/listening for these timers. This is then keeping a reference from the Timer to the Figure on every close, causing a leak when calling a bunch of savefig's.PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).