[vlc-devel] [PATCH] macosx: fix a thread-safety issue (crash when mounting multiple volumes at once)

[Colin Delacroix]

Another thread-related crash in the same area. When receiving multiple 
notifications at once, the o_opticalDevices array was used and modified at the 
same time. I see no reason to avoid passing the path object directly, which
is retained by detachNewThreadSelector, and no lock is needed at all since
the array is not accessed outside of the main thread.

---
 modules/gui/macosx/open.m |   62 +++++++++++++++++++++------------------------
 1 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/modules/gui/macosx/open.m b/modules/gui/macosx/open.m
index 0f00bea..182185e 100644
--- a/modules/gui/macosx/open.m
+++ b/modules/gui/macosx/open.m
@@ -871,62 +871,53 @@ static VLCOpen *_o_sharedMainInstance = nil;
     return returnValue;
 }
 
-- (void)showOpticalAtIndex: (NSNumber *)n_index
+- (void)showOpticalAtPath: (NSString *)o_opticalDevicePath
 {
     NSAutoreleasePool * o_pool = [[NSAutoreleasePool alloc] init];
 
-    if (!o_opticalDevices)
-        return;
-
-    if ([o_opticalDevices count] == 0)
-        return;
-
-    unsigned int index = [n_index intValue];
-
-    id o_currentOpticalDevice = [o_opticalDevices objectAtIndex: index];
-    char *diskType = [self getVolumeTypeFromMountPath:o_currentOpticalDevice];
+    char *diskType = [self getVolumeTypeFromMountPath:o_opticalDevicePath];
 
     if (diskType == kVLCMediaDVD || diskType == kVLCMediaVideoTSFolder)
     {
-        [o_disc_dvd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath:o_currentOpticalDevice]];
+        [o_disc_dvd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath:o_opticalDevicePath]];
         [o_disc_dvdwomenus_lbl setStringValue: [o_disc_dvd_lbl stringValue]];
         NSString *pathToOpen;
         if (diskType == kVLCMediaVideoTSFolder)
-            pathToOpen = o_currentOpticalDevice;
+            pathToOpen = o_opticalDevicePath;
         else
-            pathToOpen = [self getBSDNodeFromMountPath: o_currentOpticalDevice];
+            pathToOpen = [self getBSDNodeFromMountPath: o_opticalDevicePath];
         if (!b_nodvdmenus) {
             [self setMRL: [NSString stringWithFormat: @"dvdnav://%@", pathToOpen]];
-            [self showOpticalMediaView: o_disc_dvd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_currentOpticalDevice]];
+            [self showOpticalMediaView: o_disc_dvd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_opticalDevicePath]];
         } else {
             [self setMRL: [NSString stringWithFormat: @"dvdread://%@#%i:%i-", pathToOpen, [o_disc_dvdwomenus_title intValue], [o_disc_dvdwomenus_chapter intValue]]];
-            [self showOpticalMediaView: o_disc_dvdwomenus_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_currentOpticalDevice]];
+            [self showOpticalMediaView: o_disc_dvdwomenus_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_opticalDevicePath]];
         }
     }
     else if (diskType == kVLCMediaAudioCD)
     {
-        [o_disc_audiocd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath: o_currentOpticalDevice]];
-        [o_disc_audiocd_trackcount_lbl setStringValue: [NSString stringWithFormat:_NS("%i tracks"), [[[NSFileManager defaultManager] subpathsOfDirectoryAtPath: o_currentOpticalDevice error:NULL] count] - 1]]; // minus .TOC.plist
-        [self showOpticalMediaView: o_disc_audiocd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_currentOpticalDevice]];
-        [self setMRL: [NSString stringWithFormat: @"cdda://%@", [self getBSDNodeFromMountPath: o_currentOpticalDevice]]];
+        [o_disc_audiocd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath: o_opticalDevicePath]];
+        [o_disc_audiocd_trackcount_lbl setStringValue: [NSString stringWithFormat:_NS("%i tracks"), [[[NSFileManager defaultManager] subpathsOfDirectoryAtPath: o_opticalDevicePath error:NULL] count] - 1]]; // minus .TOC.plist
+        [self showOpticalMediaView: o_disc_audiocd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_opticalDevicePath]];
+        [self setMRL: [NSString stringWithFormat: @"cdda://%@", [self getBSDNodeFromMountPath: o_opticalDevicePath]]];
     }
     else if (diskType == kVLCMediaVCD)
     {
-        [o_disc_vcd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath: o_currentOpticalDevice]];
-        [self showOpticalMediaView: o_disc_vcd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_currentOpticalDevice]];
-        [self setMRL: [NSString stringWithFormat: @"vcd://%@#%i:%i", [self getBSDNodeFromMountPath: o_currentOpticalDevice], [o_disc_vcd_title intValue], [o_disc_vcd_chapter intValue]]];
+        [o_disc_vcd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath: o_opticalDevicePath]];
+        [self showOpticalMediaView: o_disc_vcd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_opticalDevicePath]];
+        [self setMRL: [NSString stringWithFormat: @"vcd://%@#%i:%i", [self getBSDNodeFromMountPath: o_opticalDevicePath], [o_disc_vcd_title intValue], [o_disc_vcd_chapter intValue]]];
     }
     else if (diskType == kVLCMediaSVCD)
     {
-        [o_disc_vcd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath: o_currentOpticalDevice]];
-        [self showOpticalMediaView: o_disc_vcd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_currentOpticalDevice]];
-        [self setMRL: [NSString stringWithFormat: @"vcd://%@@%i:%i", [self getBSDNodeFromMountPath: o_currentOpticalDevice], [o_disc_vcd_title intValue], [o_disc_vcd_chapter intValue]]];
+        [o_disc_vcd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath: o_opticalDevicePath]];
+        [self showOpticalMediaView: o_disc_vcd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_opticalDevicePath]];
+        [self setMRL: [NSString stringWithFormat: @"vcd://%@@%i:%i", [self getBSDNodeFromMountPath: o_opticalDevicePath], [o_disc_vcd_title intValue], [o_disc_vcd_chapter intValue]]];
     }
     else if (diskType == kVLCMediaBD || diskType == kVLCMediaBDMVFolder)
     {
-        [o_disc_bd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath: o_currentOpticalDevice]];
-        [self showOpticalMediaView: o_disc_bd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_currentOpticalDevice]];
-        [self setMRL: [NSString stringWithFormat: @"bluray://%@", o_currentOpticalDevice]];
+        [o_disc_bd_lbl setStringValue: [[NSFileManager defaultManager] displayNameAtPath: o_opticalDevicePath]];
+        [self showOpticalMediaView: o_disc_bd_view withIcon: [[NSWorkspace sharedWorkspace] iconForFile: o_opticalDevicePath]];
+        [self setMRL: [NSString stringWithFormat: @"bluray://%@", o_opticalDevicePath]];
     }
     else
     {
@@ -937,13 +928,18 @@ static VLCOpen *_o_sharedMainInstance = nil;
     [o_pool release];
 }
 
+- (void)showSelectedOpticalDisc
+{
+    NSString *o_opticalDevicePath = [o_opticalDevices objectAtIndex:[o_disc_selector_pop indexOfSelectedItem]];
+    [NSThread detachNewThreadSelector:@selector(showOpticalAtPath:) toTarget:self withObject:o_opticalDevicePath];
+}
+
 - (void)scanOpticalMedia:(NSNotification *)o_notification
 {
     [o_opticalDevices removeAllObjects];
     [o_disc_selector_pop removeAllItems];
     [o_opticalDevices addObjectsFromArray: [[NSWorkspace sharedWorkspace] mountedRemovableMedia]];
-    if ([o_specialMediaFolders count] > 0)
-        [o_opticalDevices addObjectsFromArray: o_specialMediaFolders];
+    [o_opticalDevices addObjectsFromArray: o_specialMediaFolders];
     if ([o_opticalDevices count] > 0) {
         NSUInteger deviceCount = [o_opticalDevices count];
         for (NSUInteger i = 0; i < deviceCount ; i++)
@@ -954,7 +950,7 @@ static VLCOpen *_o_sharedMainInstance = nil;
         else
             [o_disc_selector_pop setHidden: NO];
 
-        [NSThread detachNewThreadSelector:@selector(showOpticalAtIndex:) toTarget:self withObject:[NSNumber numberWithInt:[o_disc_selector_pop indexOfSelectedItem]]];
+        [self showSelectedOpticalDisc];
     }
     else
     {
@@ -966,7 +962,7 @@ static VLCOpen *_o_sharedMainInstance = nil;
 
 - (IBAction)discSelectorChanged:(id)sender
 {
-    [NSThread detachNewThreadSelector:@selector(showOpticalAtIndex:) toTarget:self withObject:[NSNumber numberWithInt:[o_disc_selector_pop indexOfSelectedItem]]];
+    [self showSelectedOpticalDisc];
 }
 
 - (IBAction)openSpecialMediaFolder:(id)sender
-- 
1.7.4.4