Skip to content

Fix thread sanitizer leak when launching Python 3.13 and using from neuron import h, gui.#3243

Merged
nrnhines merged 14 commits into
masterfrom
hines/gui-thread-leak
Dec 2, 2024
Merged

Fix thread sanitizer leak when launching Python 3.13 and using from neuron import h, gui.#3243
nrnhines merged 14 commits into
masterfrom
hines/gui-thread-leak

Conversation

@nrnhines

Copy link
Copy Markdown
Member

No description provided.

@nrnhines nrnhines requested a review from JCGoran November 22, 2024 13:03
@codecov

codecov Bot commented Nov 22, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 53.84615% with 12 lines in your changes missing coverage. Please review.

Project coverage is 67.08%. Comparing base (17db974) to head (c579837).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
share/lib/python/neuron/gui.py 60.00% 6 Missing ⚠️
src/nrnpython/inithoc.cpp 45.45% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3243      +/-   ##
==========================================
- Coverage   67.08%   67.08%   -0.01%     
==========================================
  Files         571      571              
  Lines      111095   111116      +21     
==========================================
+ Hits        74529    74540      +11     
- Misses      36566    36576      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nrnhines nrnhines marked this pull request as draft November 22, 2024 13:42
@azure-pipelines

Copy link
Copy Markdown

✔️ be19ebe -> Azure artifacts URL

@JCGoran JCGoran left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable, and it seems to fix the thread leak issue, I have only minor comments.

Comment thread share/lib/python/neuron/gui.py
Comment thread src/nrnpython/inithoc.cpp Outdated
@bbpbuildbot

This comment has been minimized.

@azure-pipelines

Copy link
Copy Markdown

✔️ b739947 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

@azure-pipelines

Copy link
Copy Markdown

✔️ 1bd8751 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

Comment thread share/lib/python/neuron/gui.py Outdated
@nrnhines nrnhines marked this pull request as ready for review November 30, 2024 21:06
@azure-pipelines

Copy link
Copy Markdown

✔️ 78b971d -> Azure artifacts URL

@sonarqubecloud

sonarqubecloud Bot commented Dec 2, 2024

Copy link
Copy Markdown

Please retry analysis of this Pull-Request directly on SonarQube Cloud

@azure-pipelines

Copy link
Copy Markdown

✔️ c579837 -> Azure artifacts URL

@nrnhines nrnhines merged commit e0ab555 into master Dec 2, 2024
@nrnhines nrnhines deleted the hines/gui-thread-leak branch December 2, 2024 14:54
nrnhines added a commit that referenced this pull request Mar 16, 2025
…euron import h, gui. (#3243)

* nrnpython_finalize can only cleanup and Py_Finalize from MainThread

* save stdin terminal settings on import hoc and restore on h.quit()
nrnhines added a commit that referenced this pull request Mar 21, 2025
…euron import h, gui. (#3243)

* nrnpython_finalize can only cleanup and Py_Finalize from MainThread

* save stdin terminal settings on import hoc and restore on h.quit()
nrnhines added a commit that referenced this pull request Mar 24, 2025
At starting point, jelic/8.2-py313 #3319 had only two CI failures.

Almost the entirety of this PR is to get 8.2 to pass CI.
In addition it supports Python3.13.
Bug fixes adopted from the master are
#3239 Launching nrniv -python with Python 3.13.0 does not allow use of gui.
#3259 On h.quit() terminal settings are same as when neuron.hoc was imported.
#3243 save stdin terminal settings on import hoc and restore on h.quit()
#3276 Python 3.13.1 broke [s for s in sl] where sl is a SectionList. Manual partial cherry-pick.

Other PR attempts at fixing CI failures that can be closed when this PR is merged, are #3329 #3325 #3319 #3317

Many of the CI fixes are backports from #3028 #3040 #3303 #3105 #3278

bldnrnmacpkgcmake.sh was updated. It isolates a change (avoid breaking CI) with -DNRN_MAC_PKG=ON
---------

Co-authored-by: Goran Jelic-Cizmek <goran.jelic-cizmek@epfl.ch>
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.

3 participants