Conversation
|
@dmitriyse, thanks! @vmuriart, @tonyroberts, @hsoft, @cgohlke and @tiran, please review this. |
Codecov Report
@@ Coverage Diff @@
## master #532 +/- ##
==========================================
+ Coverage 76.99% 77.03% +0.03%
==========================================
Files 64 65 +1
Lines 5612 5747 +135
Branches 888 911 +23
==========================================
+ Hits 4321 4427 +106
- Misses 1002 1016 +14
- Partials 289 304 +15
Continue to review full report at Codecov.
|
0212233 to
bfb6e59
Compare
…oduces bugs when CPython freeing up enough objects.
be8154f to
50fb157
Compare
src/runtime/pyobject.cs
Outdated
| } | ||
| } | ||
| } | ||
| catch |
There was a problem hiding this comment.
isn't is better to explicitly fail, instead of "bypassing" the exception handling with an empty catch {}? What issue is this solving that disposed and disposing flags are not taking care of?
|
|
||
| namespace Python.Runtime | ||
| { | ||
| internal class PyReferenceDecrementer : IDisposable |
There was a problem hiding this comment.
please explain (in the comments) the purpose of PyReferenceDecrementer and what issues it addressed?
| ////Marshal.FreeHGlobal(_programName); | ||
| ////_programName = IntPtr.Zero; | ||
| ////Marshal.FreeHGlobal(_pythonPath); | ||
| ////_pythonPath = IntPtr.Zero; |
src/runtime/pythonengine.cs
Outdated
|
|
||
| protected virtual void Dispose(bool disposing) | ||
| { | ||
| //ReleeaseLock is thread bound and if it's called in finalizer thread it can wrongly release lock. |
|
@dmitriyse @filmor i left some comments here, but I generally approve this pull request: |
den-run-ai
left a comment
There was a problem hiding this comment.
Please resolve merge conflicts
82c19b6 to
8a107f3
Compare
8a107f3 to
37c2449
Compare
7fd6fc4 to
80ff754
Compare
|
@dmitriyse can you please resolve one merge conflict? can you remind me the order of PRs? If I don't get any feedback from @filmor @vmuriart @yagweb, then I'm going review again and merge this within 2 weeks, when I plan to be on vacation. |
|
I will answer some day latter. Sorry. |
What does this implement/fix? Explain your changes.
Implements really working finalizers solution.
...
Does this close any currently open issues?
Probably some issues will be fixed.
...
Any other comments?
Current finalizers solution are totally not working. Probably interop calls are disallowed in the C# finalizers. Or Finalizer thread becomes broken after a few calls to ~PyObject.
...
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG