HPyGlobal: should we fail loading module multiple times in one interpreter if the module uses HPyGlobal

[steve-s]

HPyGlobal does not provide isolation for multiple instances of the same module loaded in one interpreter. However, experienced user could use HPyGlobal in such scenario, for example:

HPy type = HPyGlobal_Load(ctx, &MyType_Global);
if (HPy_IsNull(type)) {
    type = HPyType_FromSpec(...);
    HPyGlobal_Store(ctx, type);
}
HPy_Close(ctx, type);

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).

[fangerer]

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.

[fangerer]

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:

1. We could provide HPyGlobal_IsNull(...) (which means we kind of expose the NULL value in the ABI just like HPy_NULL; but I think that is fine)
2. we use 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.