Merge SDL3 upstream #62
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lue. And SDL_IterateHashTableKey is only necessary for stackable hashtables, since non-stackable ones can either iterate each unique key/value pair with SDL_IterateHashTable, or get a specific key/value pair by using SDL_FindInHashTable.
…sts. All devices are in a single hash, whether playback or capture, or physical or logical. Lookups are keyed on device ID and map to either `SDL_AudioDevice *` for physical devices or `SDL_LogicalAudioDevice *` for logical devices (as an implementation detail, you can determine which object type you have by checking a specific bit in the device ID). This simplifies a bunch of code, makes some cases significantly more efficient, and solves the problem of having to lock each physical device while the device list rwlock is held to find logical devices by ID. Device IDs hash perfectly evenly, too, being incrementing integers.
…ree'd. Reference Issue #8386.
This is a race condition if the hashtable isn't protected by a mutex, and it makes a read/write operation out of something what appears to be read-only, which is dangerously surprising from an interface viewpoint. The downside is that if you have an item that is frequently accessed that isn't in the first slot of a bucket, each find operation will take longer instead of common items bubbling to the front of the bucket. Then again, if you have several common things being looked up in rotation, they'll just be doing unnecessary shuffling here. In this case, it might be better to just use a larger hashtable or a better hashing function (or just look up the thing you need once instead of multiple times). Fixes #8391.
- No more tapdance to either join the audio device thread or have it detach itself. Significant simplication of and fixes to the locking code to prevent deadlocks. - Physical devices now keep a refcount. Each logical device increments it, as does the existence of a device thread, etc. Last unref destroys the device and takes it out of the device_hash. Since there's a lot of moving parts that might be holding a reference to a physical device, this seemed like a safer way to protect the object. - Disconnected devices now continue to function as zombie devices. Playback devices will still consume data (and just throw it away), and capture devices will continue to produce data (which always be silence). This helps apps that don't handle disconnect events; the device still stops playing/capturing, but bound audio streams will still consume data so they don't allocate more data infinitely, and apps that depend on an audio callback firing regularly to make progress won't hang. Please note that disconnected audio devices must now be explicitly closed! They always _should_ have been, but before this commit, SDL3 would destroy the disconnected device for you (and manually closing afterwards was a safe no-op). Reference Issue #8331. Fixes #8386. (and probably others).
This happens to work because our current textures are all 128x128, but in theory one shouldn't hit this case anyhow...right?! Reference Issue #8344.
HIDAPI joystick drivers may call HIDAPI_JoystickDisconnected() in their UpdateDevice() function during HIDAPI_JoystickOpen(). If they do this today, the opened joystick will end up partially initialized (no name, path, mapping GUID, etc.) because HIDAPI_GetDeviceByIndex() will no longer be able to find the SDL_HIDAPI_Device for the removed joystick. Worse still, joystick->hwdata->device becomes a dangling freed pointer the next time HIDAPI_UpdateDeviceList() is called. This leads to a UAF when the application or SDL calls SDL_JoystickClose() on this joystick. Fix all this by checking if the device no longer has any associated joysticks after calling UpdateDevice() and failing the open call if so.
The default device handles aren't free'd anywhere, so don't strdup them; everything else is managed by the hotplug thread, which does its own freeing.
First stage happens before we destroy objects, and is generally used to shut down hotplug. The second stage is the usual deinit, which cleans up the lowlevel API, unloads shared libraries, etc.
…ared. (this was legal before, but the Android NDK wants to make sure we didn't mean for this function to be marked `static` since it didn't have a formal declaration before its definition and might only be used in the one source file.)
Both strings are _right there_ for comparing, so we can just set a flag to note the device definitely changed. Also simplified string management further; hotplug thread now makes a copy of the string before releasing the lock if there was a change event, so when the lock releases further events don't see a NULL and assume it's a new device, causing a lot of work to ripple out and decide nothing has changed, until the system stabilizes again. Now, it just does the right thing once.
It needs to be SDL_RELEASE_GENERIC, because it releases both exclusive (writer) and shared (reader) locks. Without this fix, clang's `-Wthread-safety` tests generate incorrect warnings. Reference Issue #8096.
(cherry picked from commit 62266db) (SDL3 audio backends don't have the LockDevice interfaces, so this just ended up being a comment.)
(cherry picked from commit 070f578)
(cherry picked from commit 6623c87)
(cherry picked from commit 301ee21)
(cherry picked from commit 3823ba1)
ALSA expects handles to be of type ALSA_Device, and passing the handle for the default device as a plain string causes a crash as it attempts to deference the string contents itself as a pointer to a string. Create immutable static ALSA_Device structs for the default devices and pass those as the handles. They are not placed in the hotplug list, and the audio layer doesn't attempt to free ALSA handles, so there is no need to worry about them being erroneously freed.
Almost nothing checks these return values, and there's no reason a valid lock should fail to operate. The cases where a lock isn't valid (it's a bogus pointer, it was previously destroyed, a thread is unlocking a lock it doesn't own, etc) are undefined behavior and always were, and should be treated as an application bug. Reference Issue #8096.
Make sure that we don't have the context cached as current on the current thread.
We need to check if the device is ready to close before releasing the lock, in case other things are messing with the list of logical devices.
This does a ton of work that can deadlock, because several crucial WASAPI things that we want to do in response to this will block until the notification callback has returned, so we can't call them from the handler directly, or we'll be waiting until the thing that called us returns.
These are also pretty heavyweight, don't do them from the notification thread, which can deadlock everything.
The device ID strings don't change between connects, so we need to move the old device object out of the way if it still exists as a zombie, and let the reconnected device get itself a fresh object.
Also cleaned up some potential file handle leaks when querying the zenity version
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.