- Notifications
You must be signed in to change notification settings - Fork752
Explicit marshalling of Py_SetPythonHome string.#186
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
…is explicit and will not be freed automatically.
| Assert.AreEqual(PythonEngine.PythonHome, homePath); | ||
| } | ||
| } | ||
| } |
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.
@leomeuk you can get python path using this c# code which requires python interpreter:
dynamicpyos=Py.Import("os");stringhomepath=pyos.path.dirname(pyos.path.dirname(pyos.__file__));
Note thatPythonEngine.PythonPath is also "broken" likePythonEngine.PythonHome:
> #r "C:\Python\Python27\Lib\site-packages\Python.Runtime.dll"> using Python.Runtime;> PythonEngine.Initialize()> PythonEngine.PythonPathHosting process exited with exit code -1073740940.Loading context from 'CSharpInteractive.rsp'.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.
Updating this PR and noticed this comment. I documented the issue withPythonEngine.PythonPath on#414. All theget functions were broken on64bit python.
@leomeuk I added the comment inline in code, once your commit is ready please git squash it into one logical unit (commit) per contributing guidelines. Thanks! |
Looks like this would leak the allocated string if python home was set twice? Is it safe to have Python deallocate the string allcated in .net? What happens if you deallocate it in Shutdown and set pythonhome to NULL, would that stop Python from trying to also deallocate the string? |
@leomeuk can you please make a pull request against master branch? develop branch is deleted. |
@tonyroberts I'm not worried about leaking just 1 or 2 strings, this fix is more important than a small leak. |
Updates as described in#179.
I did look at calling
Marshal.FreeHGlobal()in thePythonEngine.Shutdown()method after the call to shutdown Python is made, however it looks as though Python is doing the required memory tidy up.Unit test has been created but is written to assume Python is installed at C:\Python27.