#630 Crash (SIGSEGV) in openDisc: when the same disc is queued twice

[Walter van der Heiden]

XLD 20250302 (157.2), macOS 26A428, Apple Silicon (Mac16,5).

Summary

If the same disc is added to the ripping queue twice, XLD crashes with a null pointer dereference as soon as the first rip finishes. The second rip's remaining tasks find no XLDCDDARippingSession for their device, the nil session is not detected, and -[XLDCDDARipper openDisc:] is handed a NULL xld_cdread_t *.

Steps to reproduce

1. Insert a CD and start ripping it.
2. Add the same disc to the queue a second time (e.g. by pressing the rip button again while the first rip is still running).
3. Wait for the first rip to finish.

The second rip writes one more track (with a (1) suffix, because the target file already exists) and then XLD quits unexpectedly.

Crash

Exception: EXC_BAD_ACCESS (SIGSEGV)
Subtype:   KERN_INVALID_ADDRESS at 0x0000000000000038

Thread 0 (com.apple.main-thread) Crashed:
0  XLD             xld_cdda_disc_lastsector
1  XLD             -[XLDCDDARipper openDisc:]
2  XLD             -[XLDConverterTask beginConvert]
3  XLD             -[XLDQueue convertFinished:]
4  Foundation      __NSThreadPerformPerform
5  CoreFoundation  __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__

0x38 is the offset of tracks within xld_cdread_t; disc itself is NULL.

I have this crash three times in my own logs (2026-09-10, 2026-09-11 and 2026-09-16) with an identical backtrace, both on a release build and on a build from current trunk.

Cause

XLDCDDARippingSession.m:

+ (XLDCDDARippingSession *)createSessionForDevice:(NSString *)device
{
    XLDCDDARippingSession *session = [[XLDCDDARippingSession alloc] initWithDevicePath:device];
    [sessions setObject:session forKey:device];   // replaces any existing session
    return [session autorelease];
}


- (void)destroy
{
    xld_cdda_close(&cdread);
    [sessions removeObjectForKey:devicePath];     // removes by key, not by identity
}

Queueing the same disc twice calls createSessionForDevice: twice, so the second session replaces the first one under the same key. When the first rip completes, -[XLDQueue convertFinished:] calls destroy on the first session object, and removeObjectForKey: then unregisters the second, still-active session.

XLDCDDARipper.m:

- (BOOL)openFile:(char *)path
{
    XLDCDDARippingSession *session = [XLDCDDARippingSession sessionForDevice:path];
    if([session open] == -1) return NO;
    return [self openDisc:[session descriptor]];
}

session is now nil. Messaging nil returns 0, not -1, so the guard does not fire; [session descriptor] returns NULL; openDisc: dereferences it.

The same nil-session path can presumably be reached in other ways (a disc ejected while its tasks are still queued, for instance); queueing twice is just the easiest way to trigger it.

Suggested fix

--- a/XLD/XLDCDDARippingSession.m
+++ b/XLD/XLDCDDARippingSession.m
@@

 - (void)destroy
 {
     xld_cdda_close(&cdread);
-    [sessions removeObjectForKey:devicePath];
+    /* Only unregister if this object is still the registered session for the
+       device; a newer session for the same device must not be removed. */
+    if([sessions objectForKey:devicePath] == self)
+        [sessions removeObjectForKey:devicePath];
 }
--- a/XLD/XLDCDDARipper.m
+++ b/XLD/XLDCDDARipper.m
@@
 - (BOOL)openDisc:(xld_cdread_t *)disc
 {
     unsigned int i, k, value;
+    if(!disc) return NO;
     cdread = disc;
@@
 - (BOOL)openFile:(char *)path
 {
     XLDCDDARippingSession *session = [XLDCDDARippingSession sessionForDevice:path];
+    /* Messaging nil returns 0, not -1, so the check below would let it pass. */
+    if(!session) return NO;
     if([session open] == -1) return NO;
     return [self openDisc:[session descriptor]];
 }

I have been running with this patch applied. The task that cannot find its session now fails cleanly ("cannot open the input file") instead of taking the application down, and the other rips in the queue carry on.

-- Walter van der Heiden