Skip to content

Fix missing Incref for Namespace and Assembly #482

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 6 commits into from
Jun 14, 2017

Conversation

testrunner123
Copy link
Contributor

@testrunner123 testrunner123 commented Jun 7, 2017

Fix crash of python interpreter 3.5 64-bit in garbage collector

What does this implement/fix? Explain your changes.

If Python class is derived from .NET class and namespace or assembly attributes are set then python interpreter crashes in garbage collector. Provoke garbage collector run by:

import gc
gc.collect()

Does this close any currently open issues?

#481 and may be others

Any other comments?

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

Fix crash of python interpreter 3.5 64-bit in garbage collector
@testrunner123
Copy link
Contributor Author

Well, tests failed on mono with python 3.4. I have VM with Ubuntu 14.04 & mono 4.6 & python 3.4 64-bit.
But I can't exxecute tests, because pythonnet attempts to load "__Internal" library. I see that library exists, but not taken by Python.runttime.dll. Instead it trys to load "__Internal" DLL and take Py_IsInitialized() address. That fails in mono with exception: System.EntryPointNotFoundException : Py_IsInitialized

Any help how to get these tests runing?

@den-run-ai den-run-ai closed this Jun 7, 2017
@den-run-ai den-run-ai reopened this Jun 7, 2017
@codecov
Copy link

codecov bot commented Jun 7, 2017

Codecov Report

Merging #482 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #482      +/-   ##
==========================================
- Coverage   68.56%   68.53%   -0.03%     
==========================================
  Files          63       63              
  Lines        5344     5346       +2     
  Branches      855      855              
==========================================
  Hits         3664     3664              
- Misses       1444     1446       +2     
  Partials      236      236
Flag Coverage Δ
#setup_linux 75.71% <ø> (ø) ⬆️
#setup_windows 67.84% <0%> (-0.03%) ⬇️
Impacted Files Coverage Δ
src/runtime/typemanager.cs 71.84% <0%> (-0.61%) ⬇️

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 ce14424...4c9ff8b. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 7, 2017

Codecov Report

Merging #482 into master will increase coverage by 4.8%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #482     +/-   ##
=========================================
+ Coverage   68.34%   73.15%   +4.8%     
=========================================
  Files          64       61      -3     
  Lines        5560     5483     -77     
  Branches      894      883     -11     
=========================================
+ Hits         3800     4011    +211     
+ Misses       1496     1164    -332     
- Partials      264      308     +44
Flag Coverage Δ
#setup_linux 75.71% <ø> (ø) ⬆️
#setup_windows 72.47% <50%> (+4.79%) ⬆️
Impacted Files Coverage Δ
src/runtime/typemanager.cs 83.82% <50%> (+11.37%) ⬆️
src/runtime/converter.cs 72.75% <0%> (-6.64%) ⬇️
src/runtime/CustomMarshaler.cs 67.12% <0%> (-2.74%) ⬇️
src/runtime/importhook.cs 78.01% <0%> (-2.41%) ⬇️
src/runtime/runtime.cs 86.72% <0%> (-1.43%) ⬇️
src/runtime/exceptions.cs 79.46% <0%> (-0.9%) ⬇️
src/runtime/pythonengine.cs 78.16% <0%> (-0.53%) ⬇️
src/runtime/interop.cs 95.23% <0%> (-0.5%) ⬇️
src/runtime/interop27.cs
src/runtime/interop33.cs
... and 9 more

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 550a027...90e0a6d. Read the comment docs.

@den-run-ai
Copy link
Contributor

@testrunner123 the tests are now passing on Travis CI (Mono). I'm not sure about your "__Internal" issue, maybe @vmuriart or @dmitriyse know?

@vmuriart
Copy link
Contributor

vmuriart commented Jun 7, 2017

@denfromufa are you closing and re-openning the issues to trigger the builds? You can re-trigger the builds on Travis/AppVeyor on each one's webpage. I think AppVeyor's had a bug though, or annoyance... I think you can only retrigger all Versions instead of just a couple specific ones.

@testrunner123 did you say this fixed #422 as well?

@testrunner123
Copy link
Contributor Author

testrunner123 commented Jun 9, 2017

@vmuriart
From src\test\test_subclass.py test_base_class, test_interface, test_derived_class work here. That is also what we used for interface with our assembly.

@testrunner123
Copy link
Contributor Author

@denfromufa
Under "__Internal" issue I mean that pythonnet doesn't load python.so on Ubuntu. Instead it loads "__Internal". So impossible to run src\embed_tests on Ubuntu.

@den-run-ai
Copy link
Contributor

@testrunner123 i approved this pull request, but let's keep the original issue open due to this comment from @filmor. This bigger issue can be addressed in another pull request:

#481 (comment)

Regarding "internal" let me test this locally. There is another non-reproducible issue with "internal": #445

@filmor filmor changed the title Update typemanager.cs Fix missing Incref for Namespace and Assembly Jun 13, 2017
@filmor
Copy link
Member

filmor commented Jun 13, 2017

Before merging, could you go through the checklist?

@testrunner123
Copy link
Contributor Author

testrunner123 commented Jun 13, 2017

@denfromufa
No, it is more related #489. I could submit a patch for that issue if you wish.

@den-run-ai
Copy link
Contributor

@testrunner123
Copy link
Contributor Author

@denfromufa
Done this. I also enabled first 3 subclass tests.

@filmor filmor merged commit bfce5e2 into pythonnet:master Jun 14, 2017
testrunner123 added a commit to testrunner123/pythonnet that referenced this pull request Sep 22, 2017
Fix crash of python interpreter 3.5 64-bit in garbage collector
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