Title: Fixes to integrate pythonnet into Unity by benoithudson · Pull Request #745 · pythonnet/pythonnet · GitHub
Open Graph Title: Fixes to integrate pythonnet into Unity by benoithudson · Pull Request #745 · pythonnet/pythonnet
X Title: Fixes to integrate pythonnet into Unity by benoithudson · Pull Request #745 · pythonnet/pythonnet
Description: What does this implement/fix? Explain your changes. This patch implements a fix to two successive crashes when integrating pythonnet into Unity3d, see #714 (1) On domain reload, shut down pythonnet. Otherwise there'll be a bunch of objects in python that point to functions implemented in C# code that isn't there anymore. In particular, the trick of replacing the import function means that on an import statement in the second domain, python is calling into deallocated memory. Having fixed that crash we see a second crash, fixed by: (2) Implement the garbage collection support in native code in memory that is not deallocated. Python's garbage collector keeps pointers to python objects that were allocated in the first domain and leaked on Py_Finalize. Python doesn't clear that list in Py_Initialize. This means it will try to collect these objects in the second domain. This led to a crash because garbage collection support for the python objects was implemented in C#, so on the first gc in the second domain, python was calling into deallocated memory. Support for number 2: (3) Use python's platform module to determine the platform at runtime so we know what native code to build and how to allocate an executable page. I'm preferring to check at runtime rather than at compile time so we can get away from shipping a .net DLL per configuration. (4) Unit tests for domain reload, the platform module, and for allocating a page of memory. Support is currently windows, linux, darwin; i386 and x86_64. Unit tests will fail with a descriptive message on other platforms. Does this close any currently open issues? #714 . Any other comments? It will be nice to fix a lot of the leaks, which I see some action on. But we can't rely on there being no leaks: client C# code can always XIncref some of their own stuff one too many times (or client C++ code). Best not to have everything come crashing down with inexplicable errors if client code made that mistake. It seems like the only reason we really need the native code page hack is that we use the size field for special magic stuff in metatype.cs: // Hmm - the standard subtype_traverse, clear look at ob_size to // do things, so to allow gc to work correctly we need to move // our hidden handle out of ob_size. Then, in theory we can // comment this out and still not crash. If we can fix that, we can avoid leaking a page per domain reload, and it'll be easier to port to other platforms. Checklist Check all those that are applicable and complete. [ x ] Make sure to include one or more tests for your change [ n/a ] If an enhancement PR, please create docs and at best an example [ x ] Add yourself to AUTHORS [ x ] Updated the CHANGELOG
Open Graph Description: What does this implement/fix? Explain your changes. This patch implements a fix to two successive crashes when integrating pythonnet into Unity3d, see #714 (1) On domain reload, shut down pythonnet...
X Description: What does this implement/fix? Explain your changes. This patch implements a fix to two successive crashes when integrating pythonnet into Unity3d, see #714 (1) On domain reload, shut down pythonnet...
Opengraph URL: https://github.com/pythonnet/pythonnet/pull/745
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/checks(.:format) |
| route-controller | pull_requests |
| route-action | checks |
| fetch-nonce | v2:1cda5b49-fa8c-daa6-047d-e5fc4cf5cbcd |
| current-catalog-service-hash | 87dc3bc62d9b466312751bfd5f889726f4f1337bdff4e8be7da7c93d6c00a25a |
| request-id | AD16:21CAB6:49FDB5:6857FC:69724680 |
| html-safe-nonce | c67fa7469d489292b80436b8208529a2b978eb6e4b88f19a9a6a75d461c1c058 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJBRDE2OjIxQ0FCNjo0OUZEQjU6Njg1N0ZDOjY5NzI0NjgwIiwidmlzaXRvcl9pZCI6Ijg2NDc4OTQ5NTgzODUyODQ3MzYiLCJyZWdpb25fZWRnZSI6ImlhZCIsInJlZ2lvbl9yZW5kZXIiOiJpYWQifQ== |
| visitor-hmac | 701b89ff8fac0812db719933b9409288e93cc00d9ee9c75981192b2a5b96b183 |
| hovercard-subject-tag | pull_request:219844047 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,checks,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/pythonnet/pythonnet/pull/745/checks |
| twitter:image | https://avatars.githubusercontent.com/u/15807877?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/15807877?s=400&v=4 |
| og:image:alt | What does this implement/fix? Explain your changes. This patch implements a fix to two successive crashes when integrating pythonnet into Unity3d, see #714 (1) On domain reload, shut down pythonnet... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 0bf8f9c572ee803398ae08d37b5c4215b9e90e3afebbe4c722ebdecb8f680830 |
| turbo-cache-control | no-preview |
| go-import | github.com/pythonnet/pythonnet git https://github.com/pythonnet/pythonnet.git |
| octolytics-dimension-user_id | 6050430 |
| octolytics-dimension-user_login | pythonnet |
| octolytics-dimension-repository_id | 14748123 |
| octolytics-dimension-repository_nwo | pythonnet/pythonnet |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 14748123 |
| octolytics-dimension-repository_network_root_nwo | pythonnet/pythonnet |
| turbo-body-classes | logged-out env-production page-responsive full-width full-width-p-0 |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | f1a765aea9b6f5dcd4bf980043cb1434723fac64 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width