Skip to content

Commit

Permalink
Removed temporary memory from the API
Browse files Browse the repository at this point in the history 
It was intended to make the API easier to use, but various automatic garbage collection all had flaws, and making the application periodically clean up temporary memory added cognitive load to using the API, and in many cases was it was difficult to restructure threaded code to handle this.

So, we're largely going back to the original system, where the API returns allocated results and you free them.

In addition, to solve the problems we originally wanted temporary memory for:
* Short strings with a finite count, like device names, get stored in a per-thread string pool.
* Events continue to use temporary memory internally, which is cleaned up on the next event processing cycle.
  • Loading branch information
@slouken
slouken committed Jul 27, 2024
1 parent 21411c6 commit 61126ec
Show file tree
Hide file tree
Showing 100 changed files with 715 additions and 831 deletions.
38 changes: 13 additions & 25 deletions docs/README-migration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ Rather than iterating over audio devices using a device index, there are new fun
if (devices) {
for (i = 0; i < num_devices; ++i) {
SDL_AudioDeviceID instance_id = devices[i];
const char *name = SDL_GetAudioDeviceName(instance_id);
SDL_Log("AudioDevice %" SDL_PRIu32 ": %s\n", instance_id, name);
SDL_Log("AudioDevice %" SDL_PRIu32 ": %s\n", instance_id, SDL_GetAudioDeviceName(instance_id));
}
SDL_free(devices);
}
SDL_QuitSubSystem(SDL_INIT_AUDIO);
}
Expand Down Expand Up @@ -298,10 +298,6 @@ The following symbols have been renamed:
The following symbols have been removed:
* SDL_MIX_MAXVOLUME - mixer volume is now a float between 0.0 and 1.0
## SDL_clipboard.h
SDL_GetClipboardText() and SDL_GetPrimarySelectionText() return a const pointer to temporary memory, which does not need to be freed. You can use SDL_ClaimTemporaryMemory() to convert it to a non-const pointer that should be freed when you're done with it.
## SDL_cpuinfo.h
The intrinsics headers (mmintrin.h, etc.) have been moved to `<SDL3/SDL_intrin.h>` and are no longer automatically included in SDL.h.
Expand Down Expand Up @@ -458,10 +454,6 @@ The following functions have been removed:
The following enums have been renamed:
* SDL_eventaction => SDL_EventAction

## SDL_filesystem.h

SDL_GetBasePath() and SDL_GetPrefPath() return a const pointer to temporary memory, which does not need to be freed. You can use SDL_ClaimTemporaryMemory() to convert it to a non-const pointer that should be freed when you're done with it.

## SDL_gamecontroller.h

SDL_gamecontroller.h has been renamed SDL_gamepad.h, and all APIs have been renamed to match.
Expand Down Expand Up @@ -710,15 +702,13 @@ Rather than iterating over haptic devices using device index, there is a new fun
{
if (SDL_InitSubSystem(SDL_INIT_HAPTIC) == 0) {
int i, num_haptics;
const SDL_HapticID *haptics = SDL_GetHaptics(&num_haptics);
SDL_HapticID *haptics = SDL_GetHaptics(&num_haptics);
if (haptics) {
for (i = 0; i < num_haptics; ++i) {
SDL_HapticID instance_id = haptics[i];
const char *name = SDL_GetHapticNameForID(instance_id);

SDL_Log("Haptic %" SDL_PRIu32 ": %s\n",
instance_id, name ? name : "Unknown");
SDL_Log("Haptic %" SDL_PRIu32 ": %s\n", instance_id, SDL_GetHapticNameForID(instance_id));
}
SDL_free(haptics);
}
SDL_QuitSubSystem(SDL_INIT_HAPTIC);
}
Expand Down Expand Up @@ -840,7 +830,7 @@ Rather than iterating over joysticks using device index, there is a new function
{
if (SDL_InitSubSystem(SDL_INIT_JOYSTICK) == 0) {
int i, num_joysticks;
const SDL_JoystickID *joysticks = SDL_GetJoysticks(&num_joysticks);
SDL_JoystickID *joysticks = SDL_GetJoysticks(&num_joysticks);
if (joysticks) {
for (i = 0; i < num_joysticks; ++i) {
SDL_JoystickID instance_id = joysticks[i];
Expand All @@ -850,6 +840,7 @@ Rather than iterating over joysticks using device index, there is a new function
SDL_Log("Joystick %" SDL_PRIu32 ": %s%s%s VID 0x%.4x, PID 0x%.4x\n",
instance_id, name ? name : "Unknown", path ? ", " : "", path ? path : "", SDL_GetJoystickVendorForID(instance_id), SDL_GetJoystickProductForID(instance_id));
}
SDL_free(joysticks);
}
SDL_QuitSubSystem(SDL_INIT_JOYSTICK);
}
Expand Down Expand Up @@ -1034,10 +1025,6 @@ The following symbols have been renamed:

SDL_LoadFunction() now returns `SDL_FunctionPointer` instead of `void *`, and should be cast to the appropriate function type. You can define SDL_FUNCTION_POINTER_IS_VOID_POINTER in your project to restore the previous behavior.

## SDL_locale.h

SDL_GetPreferredLocales() returns a const array of locale pointers, which does not need to be freed. You can use SDL_ClaimTemporaryMemory() to convert it to a non-const pointer that should be freed when you're done with it.

## SDL_log.h

The following macros have been removed:
Expand Down Expand Up @@ -1588,7 +1575,7 @@ Rather than iterating over sensors using device index, there is a new function S
{
if (SDL_InitSubSystem(SDL_INIT_SENSOR) == 0) {
int i, num_sensors;
const SDL_SensorID *sensors = SDL_GetSensors(&num_sensors);
SDL_SensorID *sensors = SDL_GetSensors(&num_sensors);
if (sensors) {
for (i = 0; i < num_sensors; ++i) {
SDL_Log("Sensor %" SDL_PRIu32 ": %s, type %d, platform type %d\n",
Expand All @@ -1597,6 +1584,7 @@ Rather than iterating over sensors using device index, there is a new function S
SDL_GetSensorTypeForID(sensors[i]),
SDL_GetSensorNonPortableTypeForID(sensors[i]));
}
SDL_free(sensors);
}
SDL_QuitSubSystem(SDL_INIT_SENSOR);
}
Expand Down Expand Up @@ -1996,14 +1984,15 @@ Rather than iterating over displays using display index, there is a new function
{
if (SDL_InitSubSystem(SDL_INIT_VIDEO) == 0) {
int i, num_displays = 0;
const SDL_DisplayID *displays = SDL_GetDisplays(&num_displays);
SDL_DisplayID *displays = SDL_GetDisplays(&num_displays);
if (displays) {
for (i = 0; i < num_displays; ++i) {
SDL_DisplayID instance_id = displays[i];
const char *name = SDL_GetDisplayName(instance_id);

SDL_Log("Display %" SDL_PRIu32 ": %s\n", instance_id, name ? name : "Unknown");
}
SDL_free(displays);
}
SDL_QuitSubSystem(SDL_INIT_VIDEO);
}
Expand Down Expand Up @@ -2041,13 +2030,14 @@ Rather than iterating over display modes using an index, there is a new function
{
SDL_DisplayID display = SDL_GetPrimaryDisplay();
int num_modes = 0;
const SDL_DisplayMode * const *modes = SDL_GetFullscreenDisplayModes(display, &num_modes);
SDL_DisplayMode **modes = SDL_GetFullscreenDisplayModes(display, &num_modes);
if (modes) {
for (i = 0; i < num_modes; ++i) {
SDL_DisplayMode *mode = modes[i];
SDL_Log("Display %" SDL_PRIu32 " mode %d: %dx%d@%gx %gHz\n",
display, i, mode->w, mode->h, mode->pixel_density, mode->refresh_rate);
}
SDL_free(modes);
}
}
```
Expand Down Expand Up @@ -2080,8 +2070,6 @@ SDL_WindowFlags is used instead of Uint32 for API functions that refer to window

SDL_GetWindowOpacity() directly returns the opacity instead of using an out parameter.

SDL_GetWindowICCProfile() returns a const pointer to temporary memory, which does not need to be freed. You can use SDL_ClaimTemporaryMemory() to convert it to a non-const pointer that should be freed when you're done with it.

The following functions have been renamed:
* SDL_GL_DeleteContext() => SDL_GL_DestroyContext()
* SDL_GetClosestDisplayMode() => SDL_GetClosestFullscreenDisplayMode()
Expand Down
52 changes: 0 additions & 52 deletions docs/README-strings.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,55 +5,3 @@
Unless otherwise specified, all strings in SDL, across all platforms, are
UTF-8 encoded and can represent the full range of [Unicode](https://unicode.org).


## The SDL Get String Rule.

Did you see 'SDL_GetStringRule' in the wiki or headers? Here are the details
that aren't worth copying across dozens of functions' documentation.

tl;dr: If an SDL function returns a `const char *` string, do not modify or
free it, and if you need to save it, make a copy right away.

In several cases, SDL wants to return a string to the app, and the question
in any library that does this is: _who owns this thing?_

The answer in almost all cases, is that SDL does, but not for long.

The pointer is only guaranteed to be valid until the next time the event
queue is updated, or SDL_Quit is called.

The reason for this is memory safety.

There are several strings that SDL provides that could be freed at
any moment. For example, an app calls SDL_GetAudioDeviceName(), which returns
a string that is part of the internal audio device structure. But, while this
function is returning, the user yanks the USB audio device out of the
computer, and SDL decides to deallocate the structure...and the string!
Now the app is holding a pointer that didn't live long enough to be useful,
and could crash if accessed.

To avoid this, instead of calling SDL_free on a string as soon as it's done
with it, SDL adds the pointer to a list. This list is freed at specific
points: when the event queue is run (for ongoing cleanup) and when SDL_Quit
is called (to catch things that are just hanging around). This allows the
app to use a string without worrying about it becoming bogus in the middle
of a printf() call. If the app needs it for longer, it should copy it.

When does "the event queue run"? There are several points:

- If the app calls SDL_PumpEvents() _from any thread_.
- SDL_PumpEvents is also called by several other APIs internally:
SDL_PollEvent(), SDL_PeepEvents(), SDL_WaitEvent(),
SDL_WaitEventTimeout(), and maybe others.
- If you are using [the main callbacks](main-functions#main-callbacks-in-sdl3),
the event queue can run immediately after any of the callback functions
return.

Note that these are just guaranteed minimum lifespans; any given string
might live much longer--some might even be static memory that is _never_
deallocated--but this rule promises that the app has a safe window.

Note that none of this applies if the return value is `char *` instead of
`const char *`. Please see the specific function's documentation for how
to handle those pointers.

39 changes: 0 additions & 39 deletions docs/README-winrt.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,45 +95,6 @@ Here is a rough list of what works, and what doesn't:



Upgrade Notes
-------------

#### SDL_GetPrefPath() usage when upgrading WinRT apps from SDL 2.0.3

SDL 2.0.4 fixes two bugs found in the WinRT version of SDL_GetPrefPath().
The fixes may affect older, SDL 2.0.3-based apps' save data. Please note
that these changes only apply to SDL-based WinRT apps, and not to apps for
any other platform.

1. SDL_GetPrefPath() would return an invalid path, one in which the path's
directory had not been created. Attempts to create files there
(via fopen(), for example), would fail, unless that directory was
explicitly created beforehand.

2. SDL_GetPrefPath(), for non-WinPhone-based apps, would return a path inside
a WinRT 'Roaming' folder, the contents of which get automatically
synchronized across multiple devices. This process can occur while an
application runs, and can cause existing save-data to be overwritten
at unexpected times, with data from other devices. (Windows Phone apps
written with SDL 2.0.3 did not utilize a Roaming folder, due to API
restrictions in Windows Phone 8.0).


SDL_GetPrefPath(), starting with SDL 2.0.4, addresses these by:

1. making sure that SDL_GetPrefPath() returns a directory in which data
can be written to immediately, without first needing to create directories.

2. basing SDL_GetPrefPath() off of a different, non-Roaming folder, the
contents of which do not automatically get synchronized across devices
(and which require less work to use safely, in terms of data integrity).

Apps that wish to get their Roaming folder's path can do so either by using
SDL_GetWinRTFSPath(), or directly through the WinRT class,
Windows.Storage.ApplicationData.



Setup, High-Level Steps
-----------------------

Expand Down
26 changes: 13 additions & 13 deletions include/SDL3/SDL_audio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ extern SDL_DECLSPEC int SDLCALL SDL_GetNumAudioDrivers(void);
*
* \sa SDL_GetNumAudioDrivers
*/
extern SDL_DECLSPEC_TEMP const char * SDLCALL SDL_GetAudioDriver(int index);
extern SDL_DECLSPEC const char * SDLCALL SDL_GetAudioDriver(int index);
/* @} */

/**
Expand All @@ -428,7 +428,7 @@ extern SDL_DECLSPEC_TEMP const char * SDLCALL SDL_GetAudioDriver(int index);
*
* \since This function is available since SDL 3.0.0.
*/
extern SDL_DECLSPEC_TEMP const char * SDLCALL SDL_GetCurrentAudioDriver(void);
extern SDL_DECLSPEC const char * SDLCALL SDL_GetCurrentAudioDriver(void);

/**
* Get a list of currently-connected audio playback devices.
Expand All @@ -447,7 +447,7 @@ extern SDL_DECLSPEC_TEMP const char * SDLCALL SDL_GetCurrentAudioDriver(void);
* \param count a pointer filled in with the number of devices returned, may
* be NULL.
* \returns a 0 terminated array of device instance IDs or NULL on error; call
* SDL_GetError() for more information.
* SDL_GetError() for more information. This should be freed with SDL_free() when it is no longer needed.
*
* \threadsafety It is safe to call this function from any thread.
*
Expand All @@ -456,7 +456,7 @@ extern SDL_DECLSPEC_TEMP const char * SDLCALL SDL_GetCurrentAudioDriver(void);
* \sa SDL_OpenAudioDevice
* \sa SDL_GetAudioRecordingDevices
*/
extern SDL_DECLSPEC_TEMP const SDL_AudioDeviceID * SDLCALL SDL_GetAudioPlaybackDevices(int *count);
extern SDL_DECLSPEC_FREE SDL_AudioDeviceID * SDLCALL SDL_GetAudioPlaybackDevices(int *count);

/**
* Get a list of currently-connected audio recording devices.
Expand All @@ -475,7 +475,7 @@ extern SDL_DECLSPEC_TEMP const SDL_AudioDeviceID * SDLCALL SDL_GetAudioPlaybackD
* \param count a pointer filled in with the number of devices returned, may
* be NULL.
* \returns a 0 terminated array of device instance IDs, or NULL on failure;
* call SDL_GetError() for more information.
* call SDL_GetError() for more information. This should be freed with SDL_free() when it is no longer needed.
*
* \threadsafety It is safe to call this function from any thread.
*
Expand All @@ -484,7 +484,7 @@ extern SDL_DECLSPEC_TEMP const SDL_AudioDeviceID * SDLCALL SDL_GetAudioPlaybackD
* \sa SDL_OpenAudioDevice
* \sa SDL_GetAudioPlaybackDevices
*/
extern SDL_DECLSPEC_TEMP const SDL_AudioDeviceID * SDLCALL SDL_GetAudioRecordingDevices(int *count);
extern SDL_DECLSPEC_FREE SDL_AudioDeviceID * SDLCALL SDL_GetAudioRecordingDevices(int *count);

/**
* Get the human-readable name of a specific audio device.
Expand All @@ -501,7 +501,7 @@ extern SDL_DECLSPEC_TEMP const SDL_AudioDeviceID * SDLCALL SDL_GetAudioRecording
* \sa SDL_GetAudioRecordingDevices
* \sa SDL_GetDefaultAudioInfo
*/
extern SDL_DECLSPEC_TEMP const char * SDLCALL SDL_GetAudioDeviceName(SDL_AudioDeviceID devid);
extern SDL_DECLSPEC const char * SDLCALL SDL_GetAudioDeviceName(SDL_AudioDeviceID devid);

/**
* Get the current audio format of a specific audio device.
Expand Down Expand Up @@ -550,15 +550,15 @@ extern SDL_DECLSPEC int SDLCALL SDL_GetAudioDeviceFormat(SDL_AudioDeviceID devid
* \param devid the instance ID of the device to query.
* \param count On output, set to number of channels in the map. Can be NULL.
* \returns an array of the current channel mapping, with as many elements as
* the current output spec's channels, or NULL if default.
* the current output spec's channels, or NULL if default. This should be freed with SDL_free() when it is no longer needed.
*
* \threadsafety It is safe to call this function from any thread.
*
* \since This function is available since SDL 3.0.0.
*
* \sa SDL_SetAudioStreamInputChannelMap
*/
extern SDL_DECLSPEC_TEMP const int * SDLCALL SDL_GetAudioDeviceChannelMap(SDL_AudioDeviceID devid, int *count);
extern SDL_DECLSPEC_FREE int * SDLCALL SDL_GetAudioDeviceChannelMap(SDL_AudioDeviceID devid, int *count);

/**
* Open a specific audio device.
Expand Down Expand Up @@ -1098,7 +1098,7 @@ extern SDL_DECLSPEC int SDLCALL SDL_SetAudioStreamGain(SDL_AudioStream *stream,
* \param stream the SDL_AudioStream to query.
* \param count On output, set to number of channels in the map. Can be NULL.
* \returns an array of the current channel mapping, with as many elements as
* the current output spec's channels, or NULL if default.
* the current output spec's channels, or NULL if default. This should be freed with SDL_free() when it is no longer needed.
*
* \threadsafety It is safe to call this function from any thread, as it holds
* a stream-specific mutex while running.
Expand All @@ -1107,7 +1107,7 @@ extern SDL_DECLSPEC int SDLCALL SDL_SetAudioStreamGain(SDL_AudioStream *stream,
*
* \sa SDL_SetAudioStreamInputChannelMap
*/
extern SDL_DECLSPEC_TEMP const int * SDLCALL SDL_GetAudioStreamInputChannelMap(SDL_AudioStream *stream, int *count);
extern SDL_DECLSPEC_FREE int * SDLCALL SDL_GetAudioStreamInputChannelMap(SDL_AudioStream *stream, int *count);

/**
* Get the current output channel map of an audio stream.
Expand All @@ -1121,7 +1121,7 @@ extern SDL_DECLSPEC_TEMP const int * SDLCALL SDL_GetAudioStreamInputChannelMap(S
* \param stream the SDL_AudioStream to query.
* \param count On output, set to number of channels in the map. Can be NULL.
* \returns an array of the current channel mapping, with as many elements as
* the current output spec's channels, or NULL if default.
* the current output spec's channels, or NULL if default. This should be freed with SDL_free() when it is no longer needed.
*
* \threadsafety It is safe to call this function from any thread, as it holds
* a stream-specific mutex while running.
Expand All @@ -1130,7 +1130,7 @@ extern SDL_DECLSPEC_TEMP const int * SDLCALL SDL_GetAudioStreamInputChannelMap(S
*
* \sa SDL_SetAudioStreamInputChannelMap
*/
extern SDL_DECLSPEC_TEMP const int * SDLCALL SDL_GetAudioStreamOutputChannelMap(SDL_AudioStream *stream, int *count);
extern SDL_DECLSPEC_FREE int * SDLCALL SDL_GetAudioStreamOutputChannelMap(SDL_AudioStream *stream, int *count);

/**
* Set the current input channel map of an audio stream.
Expand Down
7 changes: 5 additions & 2 deletions include/SDL3/SDL_begin_code.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,11 @@
# endif
# endif
#endif
/* This is used to mark functions that return temporary memory */
#define SDL_DECLSPEC_TEMP SDL_DECLSPEC

/* This is used to mark functions that return memory that need to be freed with SDL_free() */
#ifndef SDL_DECLSPEC_FREE
#define SDL_DECLSPEC_FREE SDL_DECLSPEC
#endif

/* By default SDL uses the C calling convention */
#ifndef SDLCALL
Expand Down
Loading

0 comments on commit 61126ec

Please sign in to comment.