Don't load `_gl_handle` into static memory when using `--mx`

[haasn]

Currently, loader/gl.c generates code like this when using it with --mx mode:

static void* _gl_handle = NULL;

static void* glad_gl_dlopen_handle(void) {
    ....
    if (_gl_handle == NULL) {
        _gl_handle = glad_get_dlopen_handle(NAMES, sizeof(NAMES) / sizeof(NAMES[0]));
    }

    return _gl_handle;
}

int gladLoaderLoadGLContext(GladGLContext *context) {
    ...

    did_load = _gl_handle == NULL;
    handle = glad_gl_dlopen_handle();
    if (handle) {
        userptr = glad_gl_build_userptr(handle);

        version = gladLoadGLContextUserPtr(context,glad_gl_get_proc, &userptr);

        if (did_load) {
            gladLoaderUnloadGL();
        }
    }

    return version;
}

This is very racy (what if one thread calls gladLoaderUnloadGL() while another thread calls gladLoaderLoadGLContext?) and as far as I can tell, the _gl_handle variable serves no purpose here, because the only scope this handle is fully contained inside gladLoaderLoadGLContext, where it gets freed again afterwards.

As far as I can tell, the only purpose of this static variable is to be used in the case of --on-demand, in which case the handle needs to be persistent so functions can be loaded ad-hoc. So we should make its existence/use conditional on this flag.

I would go ahead and make this change, but I'm a bit confused as to why gladLoaderUnloadGL even exists in the case where on-demand is not used, because as far as I can tell, the non-ondemand loader is always designed to free the handle again after loading functions?

[Dav1dde]

I generally never supported initialization from multiple threads for multiple reasons:

• most programs are single threaded (at least for the GL parts)
• multi threading with OpenGL is hard
• multi threading is hard, if I 'officially' support it, it needs to work forever, so I always went for simpler code or code that is easier on the templating
• preferred way to initialize would be with a loader from your windowing system
• glad is almost entirely made of globals in its core
• initialization can easily be synchronized outside of glad (e.g. using a mutex or writing your own loader)

Regarding the unload function, this is a tricky and I am leaning more towards having it required even for GL, that's also one of the reasons why I added it, the other is to have a unified API across all loaders. The problem is, the loader dlsym's from a dll/so and it may very well be the case that after loading is done there are still references to the library around, e.g. OpenGL 1.1 functions on windows may have such a problem (and for GLX etc. it's not just a maybe). Glad 1 does this horribly wrong.

Unloading in general is not a big deal, it's just there in case you really have to do that (for some reason). But in most cases you need those symbols until you exit and at this point the OS takes care of everything anyways.

Regarding your PR: These changes are good, but if you actually end up on relying on that behaviour this may break again in any future version.

Can you help me understand your problem, why can't you initialize contexts in your main thread (assuming this is also where context/window creation happens)? What's your opinion about a synchronized (with a mutex) wrapper function? Do you not have access to a loader function from your windowing system?

[haasn]

Regarding the unload function, this is a tricky and I am leaning more towards having it required even for GL, that's also one of the reasons why I added it, the other is to have a unified API across all loaders.

If you want to preserve it, then shouldn't we at least change it to gladLoaderUnloadGLContext(GladGLContext) when using --mx? And then maybe the _gl_handle field could be moved into that struct, as well, if you ever end up needing to persist it. (e.g. when combining --ondemand with --mx? Is that a thing?)

Can you help me understand your problem, why can't you initialize contexts in your main thread (assuming this is also where context/window creation happens)? What's your opinion about a synchronized (with a mutex) wrapper function? Do you not have access to a loader function from your windowing system?

Sure. I am writing a library, libplacebo, which is explicitly designed to be thread-safe and absent of global state. (That is the whole reason I am interested in glad2, as it offers the --mx loader). On OpenGL, we use acquire/release callbacks and internal locking per-context. In principle, it is possible for a user to have two different OpenGL contexts, on two different threads, in the same program (perhaps the program creates two different windows?), and the API of libplacebo certainly does not prevent this.

I will admit that this is still a very theoretical risk as an OpenGL context cannot be bound to two threads simultaneously, so an ordinary API user would not accidentally stumble into this scenario under typical circumstances. It mostly just bugs me that the --mx loader, which is nominally designed to keep all of its state into the dedicated context, still accesses this static variable even though it has no need for it.

What's your opinion about a synchronized (with a mutex) wrapper function?

This would work, although it feels (to me) less elegant. I could also synchronize access to the loader internally with a global mutex on my end.

[Dav1dde]

If you want to preserve it, then shouldn't we at least change it to gladLoaderUnloadGLContext(GladGLContext) when using --mx? And then maybe the _gl_handle field could be moved into that struct, as well, if you ever end up needing to persist it.

This seems like a good solution, give every Context a *userptr, the internal loader would just use it to store the dlsym handle.

What's your opinion about a synchronized (with a mutex) wrapper function? This would work, although it feels (to me) less elegant. I could also synchronize access to the loader internally with a global mutex on my end.

Sorry I should have specified, I meant synchronizing on your end.

[haasn]

This seems like a good solution, give every Context a *userptr, the internal loader would just use it to store the dlsym handle.

It sounds like the best of all worlds here. Do you want to write a patch for this before the release? If not I can try taking a stab at it as well.

Sorry I should have specified, I meant synchronizing on your end.

Oh, yes. Of course.

[Dav1dde]

It sounds like the best of all worlds here. Do you want to write a patch for this before the release? If not I can try taking a stab at it as well.

We should change this now (before 2.0), also make the unload function context specific. Yeah please go ahead if you want to prepare a PR.

[haasn]

Looking at this code again, I think there is no point to this _gl_handle existing even in the case of using an --ondemand context, because in this case it is already redundant with the static glad_gl_internal_loader_global_userptr.handle. So I think the field can be straight up deleted. I will update my PR to show you what I mean.

[Dav1dde]

@haasn Can we consider this done?