Skip to content

Commit 395c4fa

Browse files
committed
fix(vulkan): ensure we don't leak handle on double-gloamVulkanInitialize
Also, do some stylistic cleanups for the C source. Signed-off-by: Steven Noonan <steven@uplinklabs.net>
1 parent 511858e commit 395c4fa

5 files changed

Lines changed: 215 additions & 151 deletions

File tree

src/generator/c/templates/header.h.j2

Lines changed: 76 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#define GLOAM_{{ guard }}
66
{# ---- System-header conflict guards ---------------------------------------
77
These prevent including both the platform/system GL or Vulkan headers and
8-
this gloam header, which would cause duplicate declarations. Using
8+
this gloam header, which would cause duplicate declarations. Using
99
reserved identifiers (double-underscore prefixed) requires suppressing the
1010
clang warning for that specific pragma block. #}
1111
{%- if fs.spec_name in ["gl", "gles1", "gles2", "glcore"] %}
@@ -238,10 +238,11 @@ extern "C" {
238238
#endif
239239

240240
/* Calling convention for GL callback function pointers (e.g. debug callbacks,
241-
blob cache functions). Piggybacks on APIENTRY if already defined so that
242-
code which defines APIENTRY before including this header gets consistent
243-
behaviour. Guard against redefinition so multiple gloam headers included
244-
in the same translation unit don't conflict. */
241+
* blob cache functions). Piggybacks on APIENTRY if already defined so that
242+
* code which defines APIENTRY before including this header gets consistent
243+
* behaviour. Guard against redefinition so multiple gloam headers included
244+
* in the same translation unit don't conflict.
245+
*/
245246
#ifndef GLOAM_API_PTR
246247
# ifdef APIENTRY
247248
# define GLOAM_API_PTR APIENTRY
@@ -265,18 +266,20 @@ struct _cl_event;
265266
{%- if not (fs.is_vulkan and external_headers) -%}
266267
{%- if fs.features | length > 0 -%}
267268
/* ---- Version feature guards ----------------------------------------------
268-
These mirror the upstream vulkan_core.h / gl.h definitions so that code
269-
guarded by e.g. #ifdef GL_VERSION_3_3 compiles correctly against this
270-
header. */
269+
* These mirror the upstream vulkan_core.h / gl.h definitions so that code
270+
* guarded by e.g. #ifdef GL_VERSION_3_3 compiles correctly against this
271+
* header.
272+
*/
271273
{% for feat in fs.features -%}
272274
#define {{ feat.full_name }} 1
273275
{% endfor -%}
274276
{% endif -%}
275277
{%- if fs.ext_guard_groups | length > 0 %}
276278
/* ---- Extension compile-time guards ---------------------------------------
277-
These mirror the definitions in standard glext.h/gl2ext.h/eglext.h so
278-
that code guarded by e.g. #ifdef GL_ARB_draw_indirect compiles correctly
279-
against this header. */
279+
* These mirror the definitions in standard glext.h/gl2ext.h/eglext.h so
280+
* that code guarded by e.g. #ifdef GL_ARB_draw_indirect compiles correctly
281+
* against this header.
282+
*/
280283
{% for group in fs.ext_guard_groups -%}
281284
{{- u.protect_begin(group.protect) -}}
282285
{% for ext in group.items -%}
@@ -330,8 +333,9 @@ typedef enum {{ group.name }} {
330333
{% endfor -%}
331334
{% endif %}
332335
/* ---- Types ----------------------------------------------------------------
333-
Emitted in topological dependency order. Consecutive types sharing the
334-
same platform guard are coalesced into a single #ifdef/#endif block. */
336+
* Emitted in topological dependency order. Consecutive types sharing the
337+
* same platform guard are coalesced into a single #ifdef/#endif block.
338+
*/
335339
{% for group in fs.type_groups -%}
336340
{% if group.protect | length > 0 -%}
337341
#if defined({{ group.protect | join(") && defined(") }})
@@ -369,14 +373,15 @@ typedef {{ cmd.return_type }} (VKAPI_PTR *{{ cmd.pfn_type }})({{ cmd.params_str
369373
{%- endif %}
370374

371375
/* ---- Context struct ------------------------------------------------------
372-
Three anonymous unions give both indexed (array) and named (struct member)
373-
access to the same memory, at zero runtime cost.
374-
375-
featArray / extArray: unsigned char flags, one per feature / extension.
376-
pfnArray: function pointer slots, one per command.
377-
378-
Anonymous structs inside the unions are a C11 / GCC extension; they are
379-
universally supported on our target compilers. */
376+
* Three anonymous unions give both indexed (array) and named (struct member)
377+
* access to the same memory, at zero runtime cost.
378+
*
379+
* featArray / extArray: unsigned char flags, one per feature / extension.
380+
* pfnArray: function pointer slots, one per command.
381+
*
382+
* Anonymous structs inside the unions are a C11 / GCC extension; they are
383+
* universally supported on our target compilers.
384+
*/
380385
typedef struct {{ fs.context_name }} {
381386
union {
382387
unsigned char featArray[{{ fs.features | length }}];
@@ -418,22 +423,27 @@ typedef struct {{ fs.context_name }} {
418423
{%- endfor %}
419424
};
420425
};
421-
/* Built-in loader library handle. Set by gloamVulkanInitialize*Context /
422-
gloamLoaderLoad*Context when it opens the platform library. If the
423-
caller pre-populates this field, the open is skipped and ownership is
424-
NOT taken (gloam will not close it). Present on all context types so
425-
user code can use a single field name regardless of API. */
426+
427+
/* Built-in loader library handle. Set by gloamVulkanInitialize*Context /
428+
* gloamLoaderLoad*Context when it opens the platform library. If the
429+
* caller pre-populates this field, the open is skipped and ownership is
430+
* NOT taken (gloam will not close it). Present on all context types so
431+
* user code can use a single field name regardless of API.
432+
*/
426433
void *gloam_loader_handle;
434+
427435
/* Non-zero if gloam opened the library handle itself and is responsible
428-
for closing it in gloamVulkanFinalize / gloamLoaderUnload. */
436+
* for closing it in gloamVulkanFinalize / gloamLoaderUnload.
437+
*/
429438
uint8_t gloam_loader_owns_handle;
430439
{% if fs.is_vulkan %}
431440
/* The last VkInstance this context loaded entry points from */
432441
VkInstance vk_loaded_instance;
433442
/* The last VkDevice this context loaded entry points from */
434443
VkDevice vk_loaded_device;
435444
/* Vulkan discovery-path metadata — used by gloamLoaderLoadVulkanContext to make
436-
repeated calls additive without re-enumerating already-cached scopes. */
445+
* repeated calls additive without re-enumerating already-cached scopes.
446+
*/
437447
uint32_t vk_instance_version; /* cached EnumerateInstanceVersion result */
438448
uint32_t vk_device_version; /* cached GetPhysicalDeviceProperties.apiVersion */
439449
uint8_t vk_found_instance_exts; /* set once instance extensions enumerated */
@@ -442,11 +452,13 @@ typedef struct {{ fs.context_name }} {
442452
} {{ fs.context_name }};
443453

444454
/* Global context instance — a value, not a pointer, so the compiler knows
445-
its address is fixed and does not re-load it on every access. */
455+
* its address is fixed and does not re-load it on every access.
456+
*/
446457
extern {{ fs.context_name }} gloam_{{ fs.spec_name }}_context;
447458

448459
/* ---- Feature presence macros --------------------------------------------
449-
Test whether a versioned feature was detected at load time. */
460+
* Test whether a versioned feature was detected at load time.
461+
*/
450462
{%- for feat in fs.features %}
451463
#define GLOAM_{{ feat.full_name }} (gloam_{{ fs.spec_name }}_context.{{ feat.short_name }})
452464
{%- endfor %}
@@ -463,7 +475,7 @@ extern {{ fs.context_name }} gloam_{{ fs.spec_name }}_context;
463475
/* ---- Dispatch ------------------------------------------------------------ */
464476
{% if fs.is_vulkan -%}
465477
{# Vulkan: force-inlined function wrappers that dispatch through the global
466-
context struct. Unlike #define macros, these don't pollute the preprocessor
478+
context struct. Unlike #define macros, these don't pollute the preprocessor
467479
namespace (avoids collisions with VMA, etc.). #}
468480
{% for group in fs.cmd_pfn_groups -%}
469481
{% if group.protect | length > 0 -%}
@@ -512,7 +524,8 @@ GLOAM_FORCE_INLINE {{ cmd.return_type }} {{ cmd.name }}({{ cmd.params_str }}) {
512524
#ifndef GLOAM_DEFINED_CALLBACK_TYPES_
513525
#define GLOAM_DEFINED_CALLBACK_TYPES_
514526
/* Opaque function pointer type — the common return type for all load
515-
callbacks. Callers cast to the specific PFN type they need. */
527+
* callbacks. Callers cast to the specific PFN type they need.
528+
*/
516529
typedef void (*GloamAPIProc)(void);
517530

518531
/* Load function pointer type (GL / EGL / GLX / WGL). */
@@ -533,31 +546,32 @@ int gloamLoad{{ api | api_display }}Context({{ u.ctx_arg(', ') }}HDC hdc, GloamL
533546
int gloamLoad{{ api | api_display }}(HDC hdc, GloamLoadFunc getProcAddr);
534547
{%- elif fs.spec_name == "vk" %}
535548
/* ---- Vulkan enabled API (Volk-like) ----------------------------------------
536-
Phased loading: Initialize → LoadInstance → LoadDevice.
537-
The caller owns extension discovery and tells gloam what was enabled.
538-
539-
Phase 0 — Initialize: open libvulkan and load the handful of Global-scope
540-
PFNs needed to create an instance (vkCreateInstance, vkEnumerateInstance*).
541-
If library_handle is non-NULL, use it without taking ownership; if NULL,
542-
dlopen the platform default and take ownership.
543-
544-
Phase 1 — LoadInstance: load Global + Instance-scope PFNs for core features
545-
and enabled instance extensions. Set featArray from api_version
546-
(VK_MAKE_API_VERSION or VK_API_VERSION_x_y). Set extArray for enabled
547-
instance extensions. Resolve aliases.
548-
549-
Phase 1.5 (optional) — LoadPhysicalDeviceExtension(s): pre-load
550-
Instance-scope PFNs for device extensions the application wants to query
551-
before creating a VkDevice (e.g. vkGetPhysicalDeviceFragmentShadingRatesKHR
552-
from VK_KHR_fragment_shading_rate). Does NOT set extArray.
553-
554-
Phase 2 — LoadDevice: load PFNs for enabled device extensions (all scopes).
555-
Device-scope commands use vkGetDeviceProcAddr for the fast path; Instance-
556-
scope commands in device extensions use vkGetInstanceProcAddr. Update
557-
featArray from the device's api_version. Set extArray for enabled device
558-
extensions. Resolve aliases.
559-
560-
Finalize: close library handle if gloam owns it, zero the context. */
549+
* Phased loading: Initialize → LoadInstance → LoadDevice.
550+
* The caller owns extension discovery and tells gloam what was enabled.
551+
*
552+
* Phase 0 — Initialize: open libvulkan and load the handful of Global-scope
553+
* PFNs needed to create an instance (vkCreateInstance, vkEnumerateInstance*).
554+
* If library_handle is non-NULL, use it without taking ownership; if NULL,
555+
* dlopen the platform default and take ownership.
556+
*
557+
* Phase 1 — LoadInstance: load Global + Instance-scope PFNs for core features
558+
* and enabled instance extensions. Set featArray from api_version
559+
* (VK_MAKE_API_VERSION or VK_API_VERSION_x_y). Set extArray for enabled
560+
* instance extensions. Resolve aliases.
561+
*
562+
* Phase 1.5 (optional) — LoadPhysicalDeviceExtension(s): pre-load
563+
* Instance-scope PFNs for device extensions the application wants to query
564+
* before creating a VkDevice (e.g. vkGetPhysicalDeviceFragmentShadingRatesKHR
565+
* from VK_KHR_fragment_shading_rate). Does NOT set extArray.
566+
*
567+
* Phase 2 — LoadDevice: load PFNs for enabled device extensions (all scopes).
568+
* Device-scope commands use vkGetDeviceProcAddr for the fast path; Instance-
569+
* scope commands in device extensions use vkGetInstanceProcAddr. Update
570+
* featArray from the device's api_version. Set extArray for enabled device
571+
* extensions. Resolve aliases.
572+
*
573+
* Finalize: close library handle if gloam owns it, zero the context.
574+
*/
561575
{% if loader -%}
562576
int gloamVulkanInitializeContext({{ u.ctx_arg(', ') }}void *library_handle);
563577
int gloamVulkanInitialize(void *library_handle);
@@ -585,10 +599,11 @@ void gloamVulkanFinalize(void);
585599

586600
{%- if loader -%}
587601
/* Built-in loader: opens the platform library if needed and calls the
588-
appropriate load function for you. Non-Vulkan loaders call the detection-
589-
based gloamLoad* functions. Vulkan loaders handle all extension detection
590-
and PFN loading internally.
591-
Each Load function may be called multiple times (additive). */
602+
* appropriate load function for you. Non-Vulkan loaders call the detection-
603+
* based gloamLoad* functions. Vulkan loaders handle all extension detection
604+
* and PFN loading internally.
605+
* Each Load function may be called multiple times (additive).
606+
*/
592607
{%- for api in fs.apis %}
593608
{%- if fs.spec_name in ["gl", "gles1", "gles2", "glcore"] %}
594609
int gloamLoaderLoad{{ api | api_display }}Context({{ u.ctx_arg() }});

src/generator/c/templates/impl_util.j2

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
#endif
1010

1111
/* GLOAM_NO_INLINE — suppress inlining on functions that should stay out of
12-
the hot path (sort, hash, etc.) to avoid code bloat at call sites. */
12+
* the hot path (sort, hash, etc.) to avoid code bloat at call sites.
13+
*/
1314
#ifdef _MSC_VER
1415
# define GLOAM_NO_INLINE __declspec(noinline)
1516
#else
@@ -20,16 +21,18 @@
2021
#define GLOAM_UNUSED(x) ((void)(x))
2122

2223
/* Contiguous run of pfnArray slots belonging to one feature or extension.
23-
Used by the range-based PFN loading loop. */
24+
* Used by the range-based PFN loading loop.
25+
*/
2426
typedef struct {
25-
uint16_t extension; /* index into featArray or extArray */
27+
uint16_t extension; /* index into featArray or extArray */
2628
uint16_t start; /* first pfnArray index in this run */
2729
uint16_t count; /* number of consecutive slots */
2830
} GloamPfnRange_t;
2931

3032
{% if alias -%}
3133
/* Bijective alias pair: if canonical slot is null but secondary is loaded
32-
(or vice versa), the loaded pointer is propagated to both slots. */
34+
* (or vice versa), the loaded pointer is propagated to both slots.
35+
*/
3336
typedef struct {
3437
uint16_t first; /* canonical (shortest name) pfnArray index */
3538
uint16_t second; /* alias pfnArray index */

src/generator/c/templates/loader.j2

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
11
{% import "utils.j2" as u with context %}
2-
{#- Built-in platform loader. Only included when --loader is passed. -#}
2+
{#- Built-in platform loader. Only included when --loader is passed. -#}
33
{% include "library.j2" %}
44
{% if fs.spec_name in ["gl", "gles1", "gles2", "glcore"] %}
55
/* ---- GL / GLES built-in loader ----------------------------------------- */
66

77
/* Transient userptr: set for the duration of a gloamLoaderLoad*Context call.
8-
Holds the library handle and — for desktop GL — the correctly-typed
9-
platform proc-addr function pointer so that the calling convention is
10-
always right. Only valid between entry and return; never accessed
11-
concurrently. */
8+
* Holds the library handle and — for desktop GL — the correctly-typed
9+
* platform proc-addr function pointer so that the calling convention is
10+
* always right. Only valid between entry and return; never accessed
11+
* concurrently.
12+
*/
1213
struct gloam_gl_load_userptr {
1314
void *handle;
1415
#if defined(GLOAM_PLATFORM_WINDOWS)
1516
/* wglGetProcAddress is WINAPI (__stdcall); must not be called through a
16-
plain cdecl function pointer. */
17+
* plain cdecl function pointer.
18+
*/
1719
GloamAPIProc (WINAPI *wgl_get_proc)(const char *);
1820
#elif !defined(__APPLE__) && !defined(__HAIKU__)
1921
GloamAPIProc (*glx_get_proc)(const char *);
@@ -22,7 +24,8 @@ struct gloam_gl_load_userptr {
2224
static struct gloam_gl_load_userptr gloam_gl_load_state;
2325

2426
/* GL desktop adapter — matches GloamLoadFunc (__cdecl), dispatches through
25-
the correctly-typed platform proc-addr pointer stored in the userptr. */
27+
* the correctly-typed platform proc-addr pointer stored in the userptr.
28+
*/
2629
static GloamAPIProc gloam_gl_get_proc(const char *name)
2730
{
2831
GloamAPIProc result = NULL;
@@ -46,7 +49,8 @@ static GloamAPIProc gloam_gl_get_proc(const char *name)
4649
}
4750

4851
/* GLES adapter: all symbols (including extensions) are exported directly from
49-
the GLES library so plain dlsym is sufficient — no platform indirection. */
52+
* the GLES library so plain dlsym is sufficient — no platform indirection.
53+
*/
5054
static GloamAPIProc gloam_gles_get_proc(const char *name)
5155
{
5256
struct gloam_gl_load_userptr *u = &gloam_gl_load_state;
@@ -164,8 +168,9 @@ static const char * const gloam_egl_lib_names[] = {
164168
};
165169

166170
/* eglGetProcAddress has its own calling convention (declared in EGL headers as
167-
PFNEGLGETPROCADDRESSPROC). Store it with the correct type so the call always
168-
uses the right convention, even when the default differs (e.g. Win32). */
171+
* PFNEGLGETPROCADDRESSPROC). Store it with the correct type so the call always
172+
* uses the right convention, even when the default differs (e.g. Win32).
173+
*/
169174
struct gloam_egl_load_userptr {
170175
void *handle;
171176
PFNEGLGETPROCADDRESSPROC get_proc_address;
@@ -261,7 +266,8 @@ static const char * const gloam_glx_lib_names[] = {
261266
};
262267

263268
/* glXGetProcAddressARB returns __GLXextFuncPtr = void(*)(void); store with the
264-
correct function-pointer type so the call goes through the right ABI. */
269+
* correct function-pointer type so the call goes through the right ABI.
270+
*/
265271
struct gloam_glx_load_userptr {
266272
void *handle;
267273
GloamAPIProc (*get_proc_address)(const char *);
@@ -353,7 +359,8 @@ void gloamLoaderReset{{ api | api_display }}(void)
353359
static const char * const gloam_wgl_lib_names[] = { "opengl32.dll" };
354360

355361
/* wglGetProcAddress is WINAPI (__stdcall); must not be stored or called through
356-
a plain cdecl pointer. Store with the correct type in a userptr struct. */
362+
* a plain cdecl pointer. Store with the correct type in a userptr struct.
363+
*/
357364
struct gloam_wgl_load_userptr {
358365
void *handle;
359366
GloamAPIProc (WINAPI *wgl_get_proc)(const char *);
@@ -447,8 +454,9 @@ void gloamLoaderReset{{ api | api_display }}(void)
447454
*
448455
* Opens the Vulkan library into context->gloam_loader_handle if it is not
449456
* already set, stores vkGetInstanceProcAddr in the context, then delegates to
450-
* gloamVulkanDiscoverContext. Follows the same additive multi-call contract
451-
* as the underlying discover function. */
457+
* gloamVulkanDiscoverContext. Follows the same additive multi-call contract
458+
* as the underlying discover function.
459+
*/
452460
int gloamLoaderLoadVulkanContext({{ u.ctx_arg(', ') }}VkInstance instance, VkPhysicalDevice physical_device, VkDevice device)
453461
{
454462
int did_open = 0;

0 commit comments

Comments
 (0)