Skip to content

Fixed reference counting for exception objects in Py.With #1062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

lostmsu
Copy link
Member

@lostmsu lostmsu commented Feb 26, 2020

What does this implement/fix? Explain your changes.

PyObjects constructed for __exit__ method referenced existing Python objects without increasing refcount appropriately, which could lead to double-free.

Does this close any currently open issues?

Not sure, but this issue might have been one of the possible causes for #1050

PyObject(s) constructed for __exit__ method referenced existing Python objects without increasing refcount appropriately, which could lead to double-free.
@codecov-io
Copy link

codecov-io commented Feb 26, 2020

Codecov Report

Merging #1062 into master will increase coverage by 0.33%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1062      +/-   ##
==========================================
+ Coverage   86.42%   86.75%   +0.33%     
==========================================
  Files           1        1              
  Lines         302      302              
==========================================
+ Hits          261      262       +1     
+ Misses         41       40       -1
Flag Coverage Δ
#setup_linux 65.56% <ø> (+0.66%) ⬆️
#setup_windows 71.52% <ø> (ø) ⬆️
Impacted Files Coverage Δ
setup.py 86.75% <0%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22735f8...caaa798. Read the comment docs.

@filmor filmor merged commit 6c03245 into pythonnet:master Feb 26, 2020
@lostmsu lostmsu deleted the bugs/1050 branch February 27, 2020 21:40
@alexhelms
Copy link
Contributor

I just wanted to comment on here to tell you that I ran into this issue and this PR was the root cause. We had some python exceptions being fired within a Py.With and it was causing memory exceptions and blowing up.

Thanks!

AlexCatarino pushed a commit to QuantConnect/pythonnet that referenced this pull request Jun 29, 2020
…1062)

PyObject(s) constructed for __exit__ method referenced existing Python objects without increasing refcount appropriately, which could lead to double-free.
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.

4 participants