Skip to content

utils: add hint to enable/disable object validity checks #13213

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

utils: add hint to enable/disable object validity checks #13213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 19 additions & 0 deletions include/SDL3/SDL_hints.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,25 @@ extern "C" {
*/
#define SDL_HINT_CAMERA_DRIVER "SDL_CAMERA_DRIVER"

/**
* A variable that controls whether object validity checks are enabled.
*
* Object validity checks prevent undefined behaviour when passing invalid
* pointers to SDL functions. A function will return invalid argument error
* if you pass pointer to unitialized/freed SDL object. You may want to
* disable these checks to improve performance.
*
* The variable can be set to the following values:
*
* - "0": Disable object validity checks
* - "1": Enable object validity checks
*
* This hint should be set before SDL is initialized.
*
* \since This hint is avaliable since ???
*/
#define SDL_HINT_CHECK_OBJECT_VALIDITY "SDL_CHECK_OBJECT_VALIDITY"

/**
* A variable that limits what CPU features are available.
*
Expand Down
35 changes: 26 additions & 9 deletions src/SDL_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ Uint32 SDL_GetNextObjectID(void)

static SDL_InitState SDL_objects_init;
static SDL_HashTable *SDL_objects;
static bool check_validity = true;

static Uint32 SDLCALL SDL_HashObject(void *unused, const void *key)
{
Expand All @@ -153,6 +154,11 @@ void SDL_SetObjectValid(void *object, SDL_ObjectType type, bool valid)
SDL_assert(object != NULL);

if (SDL_ShouldInit(&SDL_objects_init)) {
check_validity = SDL_GetHintBoolean(SDL_HINT_CHECK_OBJECT_VALIDITY, true);
if(!check_validity) {
SDL_SetInitialized(&SDL_objects_init, true);
return;
}
Copy link

@bjorn bjorn Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

In my personal opinion, it would be alright to always initialize the hash table, but just not put any objects in it. This would rather simplify the patch because:

  • We'd need only a single check on check_validity in this function (the one below, which I think is better written as early-out rather than nesting the insert/remove code).
  • We would not need to check the value of check_validity in SDL_GetObjects or SDL_SetObjectsInvalid at all. These rarely used functions would automatically not do anything with an empty hash map.

The empty hash table should have basically zero overhead anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's actually a massive overhead. It needs to lock and unlock hashtable's RWLock (which is like a half of the lookup time), calculate hash and do one iteration in find_first_item which will break because of empty hashtable.

Copy link

Choose a reason for hiding this comment

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

It seems you're talking about SDL_ObjectValid. I wasn't suggesting to remove the check_validity condition from there.

SDL_objects = SDL_CreateHashTable(0, true, SDL_HashObject, SDL_KeyMatchObject, NULL, NULL);
const bool initialized = (SDL_objects != NULL);
SDL_SetInitialized(&SDL_objects_init, initialized);
Expand All @@ -161,10 +167,12 @@ void SDL_SetObjectValid(void *object, SDL_ObjectType type, bool valid)
}
}

if (valid) {
SDL_InsertIntoHashTable(SDL_objects, object, (void *)(uintptr_t)type, true);
} else {
SDL_RemoveFromHashTable(SDL_objects, object);
if (check_validity) {
if (valid) {
SDL_InsertIntoHashTable(SDL_objects, object, (void *)(uintptr_t)type, true);
} else {
SDL_RemoveFromHashTable(SDL_objects, object);
}
}
}

Expand All @@ -174,6 +182,10 @@ bool SDL_ObjectValid(void *object, SDL_ObjectType type)
return false;
}

if (!check_validity) {
return true;
}

const void *object_type;
if (!SDL_FindInHashTable(SDL_objects, object, &object_type)) {
return false;
Expand Down Expand Up @@ -205,6 +217,9 @@ static bool SDLCALL GetOneObject(void *userdata, const SDL_HashTable *table, con

int SDL_GetObjects(SDL_ObjectType type, void **objects, int count)
{
if (!check_validity) {
return 0;
}
GetOneObjectData data = { type, objects, count, 0 };
SDL_IterateHashTable(SDL_objects, GetOneObject, &data);
return data.num_objects;
Expand Down Expand Up @@ -236,11 +251,13 @@ static bool SDLCALL LogOneLeakedObject(void *userdata, const SDL_HashTable *tabl
void SDL_SetObjectsInvalid(void)
{
if (SDL_ShouldQuit(&SDL_objects_init)) {
// Log any leaked objects
SDL_IterateHashTable(SDL_objects, LogOneLeakedObject, NULL);
SDL_assert(SDL_HashTableEmpty(SDL_objects));
SDL_DestroyHashTable(SDL_objects);
SDL_objects = NULL;
if (check_validity) {
// Log any leaked objects
SDL_IterateHashTable(SDL_objects, LogOneLeakedObject, NULL);
SDL_assert(SDL_HashTableEmpty(SDL_objects));
SDL_DestroyHashTable(SDL_objects);
SDL_objects = NULL;
}
SDL_SetInitialized(&SDL_objects_init, false);
}
}
Expand Down
Loading