Skip to content

Commit 53f90ba

Browse files
committed
apple: attempt to fix coremidi xpc crashes
1 parent 357fce8 commit 53f90ba

File tree

1 file changed

+235
-8
lines changed

1 file changed

+235
-8
lines changed

midi/drivers/coremidi.c

Lines changed: 235 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <libretro.h>
1818
#include <lists/string_list.h>
1919
#include <string/stdstring.h>
20+
#include <rthreads/rthreads.h>
2021

2122
#include "../midi_driver.h"
2223
#include "../../verbosity.h"
@@ -51,8 +52,146 @@ typedef struct
5152
MIDIEndpointRef input_endpoint; /* Selected input endpoint */
5253
MIDIEndpointRef output_endpoint; /* Selected output endpoint */
5354
coremidi_queue_t input_queue; /* Queue for incoming MIDI events */
55+
slock_t *queue_lock; /* Mutex for queue synchronization */
56+
volatile bool is_shutting_down; /* Flag to prevent callbacks during shutdown */
5457
} coremidi_t;
5558

59+
/* Global list of active driver instances for notification handling */
60+
#define MAX_MIDI_INSTANCES 8
61+
static coremidi_t *active_instances[MAX_MIDI_INSTANCES];
62+
static slock_t *instances_lock = NULL;
63+
64+
/* Clear the queue (must be called with lock held) */
65+
static void coremidi_queue_clear(coremidi_queue_t *q)
66+
{
67+
q->read_index = 0;
68+
q->write_index = 0;
69+
}
70+
71+
/* Register a driver instance in the global list */
72+
static bool register_instance(coremidi_t *d)
73+
{
74+
int i;
75+
if (!instances_lock)
76+
return false;
77+
78+
slock_lock(instances_lock);
79+
for (i = 0; i < MAX_MIDI_INSTANCES; i++)
80+
{
81+
if (!active_instances[i])
82+
{
83+
active_instances[i] = d;
84+
slock_unlock(instances_lock);
85+
return true;
86+
}
87+
}
88+
slock_unlock(instances_lock);
89+
90+
RARCH_ERR("[MIDI] Too many MIDI instances (max %d).\n", MAX_MIDI_INSTANCES);
91+
return false;
92+
}
93+
94+
/* Unregister a driver instance from the global list */
95+
static void unregister_instance(coremidi_t *d)
96+
{
97+
int i;
98+
if (!instances_lock || !d)
99+
return;
100+
101+
slock_lock(instances_lock);
102+
for (i = 0; i < MAX_MIDI_INSTANCES; i++)
103+
{
104+
if (active_instances[i] == d)
105+
{
106+
active_instances[i] = NULL;
107+
break;
108+
}
109+
}
110+
slock_unlock(instances_lock);
111+
}
112+
113+
/* MIDI notification callback for device changes */
114+
static void midi_notify_callback(const MIDINotification *message, void *refCon)
115+
{
116+
int i;
117+
118+
if (!instances_lock)
119+
return;
120+
121+
switch (message->messageID)
122+
{
123+
case kMIDIMsgObjectRemoved:
124+
{
125+
const MIDIObjectAddRemoveNotification *notify =
126+
(const MIDIObjectAddRemoveNotification *)message;
127+
128+
/* Check all active instances for matching endpoints */
129+
slock_lock(instances_lock);
130+
for (i = 0; i < MAX_MIDI_INSTANCES; i++)
131+
{
132+
coremidi_t *d = active_instances[i];
133+
if (!d || d->is_shutting_down)
134+
continue;
135+
136+
/* Check if this instance's endpoints were removed */
137+
if (notify->child == d->input_endpoint)
138+
{
139+
RARCH_LOG("[MIDI] Input endpoint removed, clearing.\n");
140+
d->input_endpoint = 0;
141+
if (d->queue_lock)
142+
{
143+
slock_lock(d->queue_lock);
144+
coremidi_queue_clear(&d->input_queue);
145+
slock_unlock(d->queue_lock);
146+
}
147+
}
148+
if (notify->child == d->output_endpoint)
149+
{
150+
RARCH_LOG("[MIDI] Output endpoint removed, clearing.\n");
151+
d->output_endpoint = 0;
152+
}
153+
}
154+
slock_unlock(instances_lock);
155+
break;
156+
}
157+
case kMIDIMsgSetupChanged:
158+
RARCH_LOG("[MIDI] MIDI setup changed.\n");
159+
break;
160+
default:
161+
break;
162+
}
163+
}
164+
165+
/* Validate that an endpoint still exists in the system */
166+
static bool coremidi_validate_endpoint(MIDIEndpointRef endpoint, bool is_source)
167+
{
168+
ItemCount i, count;
169+
170+
if (!endpoint)
171+
return false;
172+
173+
if (is_source)
174+
{
175+
count = MIDIGetNumberOfSources();
176+
for (i = 0; i < count; i++)
177+
{
178+
if (MIDIGetSource(i) == endpoint)
179+
return true;
180+
}
181+
}
182+
else
183+
{
184+
count = MIDIGetNumberOfDestinations();
185+
for (i = 0; i < count; i++)
186+
{
187+
if (MIDIGetDestination(i) == endpoint)
188+
return true;
189+
}
190+
}
191+
192+
return false;
193+
}
194+
56195
/* Write to the queue */
57196
static bool coremidi_queue_write(coremidi_queue_t *q, const midi_event_t *ev)
58197
{
@@ -77,12 +216,22 @@ static bool coremidi_queue_write(coremidi_queue_t *q, const midi_event_t *ev)
77216

78217
/* MIDIReadProc callback function */
79218
static void midi_read_callback(const MIDIPacketList *pktlist,
80-
void *readProcRefCon, void *srcConnRefCon)
219+
void *readProcRefCon, void *srcConnRefCon)
81220
{
82221
uint32_t i;
83222
midi_event_t event;
84223
coremidi_t *d = (coremidi_t *)readProcRefCon;
85-
const MIDIPacket *packet = &pktlist->packet[0];
224+
const MIDIPacket *packet;
225+
226+
/* CRITICAL: Check shutdown flag immediately to prevent use-after-free */
227+
if (!d || d->is_shutting_down)
228+
return;
229+
230+
/* Early validation */
231+
if (!d->queue_lock)
232+
return;
233+
234+
packet = &pktlist->packet[0];
86235

87236
for (i = 0; i < pktlist->numPackets; i++)
88237
{
@@ -100,8 +249,10 @@ static void midi_read_callback(const MIDIPacketList *pktlist,
100249
event.data_size = msg_size;
101250
event.delta_time = (uint32_t)timestamp;
102251

103-
/* Add to queue */
252+
/* Add to queue with lock protection */
253+
slock_lock(d->queue_lock);
104254
coremidi_queue_write(&d->input_queue, &event);
255+
slock_unlock(d->queue_lock);
105256

106257
data += msg_size;
107258
length -= msg_size;
@@ -117,11 +268,22 @@ static void *coremidi_init(const char *input, const char *output)
117268
OSStatus err;
118269
coremidi_t *d;
119270

120-
/* Create persistent client on first call */
271+
/* Initialize global instance lock on first call */
272+
if (!instances_lock)
273+
{
274+
instances_lock = slock_new();
275+
if (!instances_lock)
276+
{
277+
RARCH_ERR("[MIDI] Failed to create instances lock.\n");
278+
return NULL;
279+
}
280+
}
281+
282+
/* Create persistent shared client on first call */
121283
if (!shared_midi_client)
122284
{
123285
err = MIDIClientCreate(CFSTR("RetroArch MIDI Client"),
124-
NULL, NULL, &shared_midi_client);
286+
midi_notify_callback, NULL, &shared_midi_client);
125287
if (err != noErr)
126288
{
127289
RARCH_ERR("[MIDI] MIDIClientCreate failed: %d.\n", err);
@@ -137,9 +299,29 @@ static void *coremidi_init(const char *input, const char *output)
137299
return NULL;
138300
}
139301

302+
/* Initialize synchronization primitives */
303+
d->queue_lock = slock_new();
304+
if (!d->queue_lock)
305+
{
306+
RARCH_ERR("[MIDI] Failed to create queue lock.\n");
307+
free(d);
308+
return NULL;
309+
}
310+
311+
d->is_shutting_down = false;
312+
140313
/* Store reference to shared client */
141314
d->client = shared_midi_client;
142315

316+
/* Register this instance in the global list */
317+
if (!register_instance(d))
318+
{
319+
RARCH_ERR("[MIDI] Failed to register MIDI instance.\n");
320+
slock_free(d->queue_lock);
321+
free(d);
322+
return NULL;
323+
}
324+
143325
/* Create input port if specified */
144326
if (input)
145327
{
@@ -148,6 +330,8 @@ static void *coremidi_init(const char *input, const char *output)
148330
if (err != noErr)
149331
{
150332
RARCH_ERR("[MIDI] MIDIInputPortCreate failed: %d.\n", err);
333+
unregister_instance(d);
334+
slock_free(d->queue_lock);
151335
free(d);
152336
return NULL;
153337
}
@@ -164,6 +348,8 @@ static void *coremidi_init(const char *input, const char *output)
164348
RARCH_ERR("[MIDI] MIDIOutputPortCreate failed: %d.\n", err);
165349
if (d->input_port)
166350
MIDIPortDispose(d->input_port);
351+
unregister_instance(d);
352+
slock_free(d->queue_lock);
167353
free(d);
168354
return NULL;
169355
}
@@ -261,6 +447,14 @@ static bool coremidi_set_input(void *p, const char *input)
261447
RARCH_LOG("[MIDI] Disconnecting current input endpoint.\n");
262448
MIDIPortDisconnectSource(d->input_port, d->input_endpoint);
263449
d->input_endpoint = 0;
450+
451+
/* Clear the input queue when disconnecting */
452+
if (d->queue_lock)
453+
{
454+
slock_lock(d->queue_lock);
455+
coremidi_queue_clear(&d->input_queue);
456+
slock_unlock(d->queue_lock);
457+
}
264458
}
265459

266460
/* If input is NULL or "Off", just return success */
@@ -365,7 +559,7 @@ static bool coremidi_queue_read(coremidi_queue_t *q, midi_event_t *ev)
365559
/* Read a MIDI event */
366560
static bool coremidi_read(void *p, midi_event_t *event)
367561
{
368-
int result;
562+
bool result;
369563
coremidi_t *d = (coremidi_t *)p;
370564

371565
if (!d || !event)
@@ -380,7 +574,21 @@ static bool coremidi_read(void *p, midi_event_t *event)
380574
return false;
381575
}
382576

577+
/* Validate endpoint still exists */
578+
if (!coremidi_validate_endpoint(d->input_endpoint, true))
579+
{
580+
RARCH_WARN("[MIDI] Input endpoint no longer valid.\n");
581+
d->input_endpoint = 0;
582+
return false;
583+
}
584+
585+
if (!d->queue_lock)
586+
return false;
587+
588+
slock_lock(d->queue_lock);
383589
result = coremidi_queue_read(&d->input_queue, event);
590+
slock_unlock(d->queue_lock);
591+
384592
#if DEBUG
385593
RARCH_LOG("[MIDI] Input queue read result: %d.\n", result);
386594
#endif
@@ -404,6 +612,14 @@ static bool coremidi_write(void *p, const midi_event_t *event)
404612
return false;
405613
}
406614

615+
/* Validate endpoint still exists */
616+
if (!coremidi_validate_endpoint(d->output_endpoint, false))
617+
{
618+
RARCH_WARN("[MIDI] Output endpoint no longer valid.\n");
619+
d->output_endpoint = 0;
620+
return false;
621+
}
622+
407623
/* Validate event data */
408624
if (!event->data || event->data_size == 0 || event->data_size > 65535)
409625
{
@@ -466,7 +682,12 @@ static void coremidi_free(void *p)
466682
return;
467683
}
468684

469-
/* Clean up MIDI resources */
685+
/* CRITICAL: Set shutdown flag and unregister from global list FIRST.
686+
* This prevents callbacks from accessing this instance. */
687+
d->is_shutting_down = true;
688+
unregister_instance(d);
689+
690+
/* Now safe to tear down MIDI resources */
470691
if (d->input_port)
471692
{
472693
RARCH_LOG("[MIDI] Disconnecting and disposing input port...\n");
@@ -481,9 +702,15 @@ static void coremidi_free(void *p)
481702
MIDIPortDispose(d->output_port);
482703
}
483704

705+
/* Note: Don't dispose shared_midi_client, it's persistent */
706+
707+
/* Free synchronization primitives */
708+
if (d->queue_lock)
709+
slock_free(d->queue_lock);
484710

485-
/* Free the driver instance */
711+
/* Finally, free the driver instance */
486712
free(d);
713+
RARCH_LOG("[MIDI] CoreMIDI driver freed.\n");
487714
}
488715

489716
/* CoreMIDI driver API */

0 commit comments

Comments
 (0)