- Notifications
You must be signed in to change notification settings - Fork749
RemovedShutdownMode. Now always behaves like originalReload#1638
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
7ff467a to7ab2bfcCompareUh oh!
There was an error while loading.Please reload this page.
7ab2bfc tofd451d5Compare@filmor looks like I got tests to pass, so this is ready. |
3157cf5 to5113257CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| } | ||
| else | ||
| { | ||
| ResetPyMembers(); | ||
| if (mode != ShutdownMode.Extension) | ||
| { | ||
| Py_Finalize(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is my only real gripe with this: Embedders would not be able to finalise the Python runtime at all anymore. Maybe we should provide this (Shutdown() +Py_Finalize()) as a possible footgun with a long name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We can always add it if there is a strong request. Instead what happened is people opening bugs about crashes after restarting runtime because NumPy does not support it. I'd rather avoid it.
Moreover, the behavior of the whole thing when either of runtimes is shut down completely is pretty much undefined.
Uh oh!
There was an error while loading.Please reload this page.
8081238 to8a093c2Compare… is an equivalent of `ShutdownMode.Reload`also in this change:- fixed Python derived types not being decrefed when an instance is deallocated- reduced time and amount of storage needed for runtime reload- removed circular reference loop between Type <-> ConstructorBinding(s)+ exposed Runtime.TryCollectingGarbage
clearing GCHandle from an instance of Python derived type would drop the last reference to it, so it was destroyed without being removed from reflectedObjects collection
8a093c2 todfb87dcCompare@filmor I am waiting for an explicit approval or more comments |
What does this implement/fix? Explain your changes.
This removes all shutdown modes, except
Reload(which you don't need to specify, because it is the only one left).This means Python C runtime is never actually shut down when .NET calls
PythonEngine.Shutdown(). Instead, anything from .NET exposed to Python beforeShutdownbecomes unavailable untilPythonEngine.Initialize()is called again (which can be done from a differentAppDomain).Any other comments?
Also in this change:
ConstructorBinding(s)Runtime.TryCollectingGarbageA review from@amos402 would be welcome