Skip to content

Drop LoadLibrary #880

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.

Sign up for GitHub

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

Closed
wants to merge 14 commits into from
Closed

Drop LoadLibrary #880

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d1928dc
Drop LoadLibrary dependency
amos402 Jun 13, 2019
f000e08
Explain for getting _PyObject_NextNotImplemented
amos402 Jun 13, 2019
f882400
Update changelog and authors
amos402 Jun 13, 2019
b7715ee
Merge branch 'master' into drop-dlopen
filmor Jun 20, 2019
41ac665
Merge remote-tracking branch 'remotes/upstream/master' into drop-dlopen
amos402 Jul 1, 2019
c6dae9e
Merge branch 'master' into drop-dlopen
amos402 Jul 5, 2019
653a263
Merge branch 'master' into drop-dlopen
amos402 Dec 1, 2019
b3e889b
* Load PyModuleType without LibraryLoader
amos402 Dec 1, 2019
04d6dfb
Merge remote-tracking branch 'remotes/upstream/master' into drop-dlop…
amos402 Dec 14, 2019
5150e61
Prevent exception override
amos402 Dec 14, 2019
65e209e
Rollback symbol loading for __Internal
amos402 Dec 15, 2019
431d644
Merge branch 'master' into drop-dlopen
amos402 Feb 12, 2020
2b84394
* Fix refcnt error
amos402 Feb 12, 2020
b7797d3
Merge 'master' into drop-dlopen
amos402 Feb 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
* Load PyModuleType without LibraryLoader 
* Drop C module dependency when getting _PyObject_NextNotImplemented
* Exception details for SetNoSiteFlag
  • Loading branch information
@amos402
amos402 committed Dec 1, 2019
commit b3e889b9f6f320f7a40f67dfe6cbe315e03215bb
62 changes: 38 additions & 24 deletions src/runtime/runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,26 +292,18 @@ internal static void Initialize(bool initSigs = false)

Error = new IntPtr(-1);

_PyObject_NextNotImplemented = Get_PyObject_NextNotImplemented();
{
IntPtr sys = PyImport_ImportModule("sys");
PyModuleType = PyObject_Type(sys);
XDecref(sys);
}

// Initialize data about the platform we're running on. We need
// this for the type manager and potentially other details. Must
// happen after caching the python types, above.
InitializePlatformData();

IntPtr dllLocal = IntPtr.Zero;
var loader = LibraryLoader.Get(OperatingSystem);

// Since `_PyObject_NextNotImplemented` would set to a heap class
// for tp_iternext which doesn't implement __next__.
// Thus we need a heap class to get it, the ZipImportError is a
// heap class and it's in builtins, so we can use it as a trick.
var zipimport = PyImport_ImportModule("zipimport");
var ZipImportError = PyObject_GetAttrString(zipimport, "ZipImportError");
_PyObject_NextNotImplemented = Marshal.ReadIntPtr(ZipImportError, TypeOffset.tp_iternext);
XDecref(ZipImportError);
XDecref(zipimport);
PyModuleType = loader.GetFunction(dllLocal, "PyModule_Type");


// Initialize modules that depend on the runtime class.
AssemblyManager.Initialize();
PyCLRMetaType = MetaType.Initialize();
Expand All @@ -328,6 +320,29 @@ internal static void Initialize(bool initSigs = false)
AssemblyManager.UpdatePath();
}

private static IntPtr Get_PyObject_NextNotImplemented()
{
IntPtr globals = PyDict_New();
if (PyDict_SetItemString(globals, "__builtins__", PyEval_GetBuiltins()) != 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can PyEval_GetBuiltins() return an error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the docs it doesn't look like it can (https://docs.python.org/3/c-api/reflection.html#c.PyEval_GetBuiltins).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The globals dict is needed, but we don't seem to be using the __builtins__ in the string. Do we really need this line?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't setup a __builtins__ or set object explicit, object may not be found.

{
XDecref(globals);
throw new PythonException();
}
const string code = "class A(object): pass";
IntPtr res = PyRun_String(code, (IntPtr)RunFlagType.File, globals, globals);
if (res == IntPtr.Zero)
{
XDecref(globals);
throw new PythonException();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to construct an instance of PythonException before doing XDecref, otherwise last error could potentially be overwritten by XDecref with something new.

}
XDecref(res);
IntPtr A = PyDict_GetItemString(globals, "A");
IntPtr iternext = Marshal.ReadIntPtr(A, TypeOffset.tp_iternext);
XDecref(globals);
XDecref(A);
return iternext;
}

/// <summary>
/// Initializes the data about platforms.
///
Expand Down Expand Up @@ -1893,25 +1908,24 @@ internal static IntPtr PyMem_Realloc(IntPtr ptr, long size)

internal static void SetNoSiteFlag()
{
if (_PythonDll == "__Internal")
{
throw new NotSupportedException("SetNoSiteFlag didn't support on static compile");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should this not be supported? The symbol is just in the running DLL, so loader.Load(_PythonDll) has to return essentially the result of dlopen(NULL, ...).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should be supported. Just a mistake according to my wrong tests. I will recover it back.

}
var loader = LibraryLoader.Get(OperatingSystem);

IntPtr dllLocal;
if (_PythonDll != "__Internal")
IntPtr dllLocal = loader.Load(_PythonDll);
if (dllLocal == IntPtr.Zero)
{
dllLocal = loader.Load(_PythonDll);
throw new Exception($"Cannot load {_PythonDll}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you throw more informative exception? Win32Exception would automatically read last error on Windows, but I don't know what to do for *nix

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally it should throw inside from Load, the nullptr checking just for in case(if the loader implementation didn't throw an exception). I'm not going to insert any platform specific code here.

}

try
{
Py_NoSiteFlag = loader.GetFunction(dllLocal, "Py_NoSiteFlag");
Marshal.WriteInt32(Py_NoSiteFlag, 1);
}
finally
{
if (dllLocal != IntPtr.Zero)
{
loader.Free(dllLocal);
}
loader.Free(dllLocal);
}
}
}
Expand Down