Skip to content

Fix memory management errors in ExtensibleRate#1499

Merged
ischoegl merged 6 commits intoCantera:mainfrom
speth:fix-extensible-rate
Jun 11, 2023
Merged

Fix memory management errors in ExtensibleRate#1499
ischoegl merged 6 commits intoCantera:mainfrom
speth:fix-extensible-rate

Conversation

@speth
Copy link
Copy Markdown
Member

@speth speth commented Jun 10, 2023

Changes proposed in this pull request

  • Don't buffer output from Python when embedding, to avoid confusing ordering of debug log messages
  • Print stack traces after tests that segfault as a debugging aid
  • Update to the attempted fix for Occasional segfault in ExtensibleRate tests #1489 in 2158ff9: we need to use a weak_ptr to know whether the C++ Solution object still exists before calling removeChangedCallback.
  • Fix a reference counting error when creating a PythonHandle from Python

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

speth added 6 commits June 9, 2023 23:09
@speth
[Python] Add test for directly-instantiated ExtensibleRate objects
7c7d19d
@speth
[Python] Improved fix for cleanup of Solution change callbacks
c92b575 
Fixes a case not handled by 2158ff9 for "weak" wrappers where errors
could occur if the destructor for the weak wrapper was called after the
destructor for the owning Solution object.

Fixes Cantera#1489
@speth
[Python] Reuse existing Python wrappers for "adjacent" phases
dc0e5dd 
The code for instantiating Python Solution wrappers for phases adjacent to an
interface did not link them as the wrapper to be used by ExtensibleRate objects.
@speth
[Python] Don't buffer output from Python when embedding
cdfa26c
@speth
[Test] Enable printing stack trace after fatal errors
9664689
@speth
Fix ownership logic for PythonHandle objects
b1483f1
@speth speth added the Python label Jun 10, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 10, 2023

Codecov Report

Merging #1499 (b1483f1) into main (44c8b6d) will increase coverage by 0.00%.
The diff coverage is 60.00%.

@@           Coverage Diff           @@
##             main    #1499   +/-   ##
=======================================
  Coverage   69.49%   69.50%           
=======================================
  Files         377      377           
  Lines       57995    58011   +16     
  Branches    20693    20695    +2     
=======================================
+ Hits        40305    40318   +13     
- Misses      14739    14741    +2     
- Partials     2951     2952    +1
Impacted Files Coverage Δ
include/cantera/base/global.h 81.81% <ø> (ø)
src/base/global.cpp 76.92% <33.33%> (-4.99%) ⬇️
interfaces/cython/cantera/_utils.pyx 91.71% <100.00%> (+0.05%) ⬆️
interfaces/cython/cantera/solutionbase.pyx 89.44% <100.00%> (+0.06%) ⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ischoegl
ischoegl approved these changes Jun 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

Thanks, @speth for decoupling this from #1490. Changes all look good to me!

@ischoegl ischoegl merged commit 13cb334 into Cantera:main Jun 11, 2023
@speth speth deleted the fix-extensible-rate branch July 23, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ischoegl ischoegl ischoegl approved these changes

Assignees

No one assigned

Labels

Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@speth @ischoegl