Open steve-s opened 1 year ago
I am slightly in favor of disallowing this and failing if HPyGlobals are registered in the module and the module is loaded twice
Yes, I agree. Allowing this and silently ignoring sharing of globals could be a source of annoying errors. I still assume that loading multiple module instances in one interpreter is an infrequent scenario.
Btw. as far as I can tell, your example will crash because HPyGlobal_Load
is implemented in this way:
HPyAPI_FUNC HPy HPyGlobal_Load(HPyContext *ctx, HPyGlobal global)
{
PyObject *obj = _hg2py(global);
Py_INCREF(obj);
return _py2h(obj);
}
(same for universal)
The runtime will initialize the globals to (internally) NULL
.
We have two options to solve this:
HPyGlobal_IsNull(...)
(which means we kind of expose the NULL
value in the ABI just like HPy_NULL
; but I think that is fine)Py_XINCREF
in the implementation.It's not strictly necessary to do anything because it is still possible to have an auxiliary flag to indicate that the global was already initialized but that's from a usability point of view not nice.
HPyGlobal
does not provide isolation for multiple instances of the same module loaded in one interpreter. However, experienced user could useHPyGlobal
in such scenario, for example:I am slightly in favor of disallowing this and failing if HPyGlobals are registered in the module and the module is loaded twice in the same interpreter. It would be a compatible change to allow this when some flag is explicitly set, so we are not shutting the door - we can add this when we see a use case. (I think that people that want to support loading module multiple times in one interpreter should ideally use module state & it wasn't a thing some time ago, so I'd only expect a green-field new projects to do this).