macosx/framework: Make sure the nsobject drawable is never used after being freed.

This issue was pointed out by Sébastien Zwickert.

We ensure this by two things:
- Retaining it by the media player and ensuring we are effectively stopped when releasing it. (ie, no one will use the drawable from now on).
- Retaining it during the life span of the vout. After a stop, the drawable might still be in the progress of receiving the notification of the vout removal, so we need it not to be freed during this period of time.
An alternative would be to create a protocol between drawable->vout to unregister the drawable.

diff --git a/modules/gui/minimal_macosx/VLCOpenGLVoutView.h b/modules/gui/minimal_macosx/VLCOpenGLVoutView.h
index 406887af3dcbd1c066ab5063af0cb21bc8e2c759..3045843c6e7bf44bdb9550938259679aed7e8a6f 100644 (file)
--- a/modules/gui/minimal_macosx/VLCOpenGLVoutView.h
+++ b/modules/gui/minimal_macosx/VLCOpenGLVoutView.h
@@ -44,7 +44,7 @@ int  cocoaglvoutviewLock( vout_thread_t * p_vout );
 void cocoaglvoutviewUnlock( vout_thread_t * p_vout );
 
 /* To commmunicate with the VLC.framework */
-@protocol VLCOpenGLVoutEmbedding
+@protocol VLCOpenGLVoutEmbedding <NSObject>
 - (void)addVoutSubview:(NSView *)view;
 - (void)removeVoutSubview:(NSView *)view;
 

diff --git a/modules/gui/minimal_macosx/VLCOpenGLVoutView.m b/modules/gui/minimal_macosx/VLCOpenGLVoutView.m
index 8b50b4250c9c21b198e5e16991735bbbcbbb2314..fe6fbe36f87956101e3a629fb0d11cbc327e9931 100644 (file)
--- a/modules/gui/minimal_macosx/VLCOpenGLVoutView.m
+++ b/modules/gui/minimal_macosx/VLCOpenGLVoutView.m
@@ -54,7 +54,9 @@ int cocoaglvoutviewInit( vout_thread_t * p_vout )
 
     p_vout->p_sys->o_pool = [[NSAutoreleasePool alloc] init];
 
-    o_cocoaglview_container = (id) value_drawable.p_address;
+    /* This will be released in cocoaglvoutviewEnd(), on
+     * main thread, after we are done using it. */
+    o_cocoaglview_container = [(id) value_drawable.p_address retain];
     if (!o_cocoaglview_container)
     {
         msg_Warn( p_vout, "No drawable!, spawing a window" );
@@ -99,6 +101,9 @@ void cocoaglvoutviewEnd( vout_thread_t * p_vout )
     [p_vout->p_sys->o_glview performSelectorOnMainThread:@selector(removeFromSuperviewAndRelease) withObject:nil waitUntilDone:NO];
     p_vout->p_sys->o_glview = nil;
 
+    /* Release the container now that we don't use it */
+    [o_cocoaglview_container performSelectorOnMainThread:@selector(release) withObject:nil waitUntilDone:NO];
+
     [p_vout->p_sys->o_pool release];
     p_vout->p_sys->o_pool = nil;
  

diff --git a/projects/macosx/framework/Headers/Public/VLCMediaPlayer.h b/projects/macosx/framework/Headers/Public/VLCMediaPlayer.h
index 0da9e463d2365f52d243e469c4ce39b85fd62816..6112f4727dabf01951134772e7a591a11501c53b 100644 (file)
--- a/projects/macosx/framework/Headers/Public/VLCMediaPlayer.h
+++ b/projects/macosx/framework/Headers/Public/VLCMediaPlayer.h
@@ -83,6 +83,7 @@ extern NSString * VLCMediaPlayerStateToString(VLCMediaPlayerState state);
     VLCTime * cachedTime;               //< Cached time of the media being played
     VLCMediaPlayerState cachedState;    //< Cached state of the media being played
     float position;                     //< The position of the media being played
+    id drawable;                        //< The drawable associated to this media player
 }
 
 /* Initializers */

diff --git a/projects/macosx/framework/Sources/VLCMediaPlayer.m b/projects/macosx/framework/Sources/VLCMediaPlayer.m
index 4954260400721a8c58c3e4069f80b4e3055e8886..59095950dc0b4916b8eb22f28eb1d0786d412de2 100644 (file)
--- a/projects/macosx/framework/Sources/VLCMediaPlayer.m
+++ b/projects/macosx/framework/Sources/VLCMediaPlayer.m
@@ -192,15 +192,23 @@ static void HandleMediaInstanceStateChanged(const libvlc_event_t * event, void *
 
 - (void)dealloc
 {
+    NSAssert(libvlc_media_player_get_state(instance, NULL) == libvlc_Stopped, @"You released the media player before ensuring that it is stopped");
+
     // Always get rid of the delegate first so we can stop sending messages to it
     // TODO: Should we tell the delegate that we're shutting down?
     delegate = nil;
 
-    libvlc_media_player_release((libvlc_media_player_t *)instance);
+    // Clear our drawable as we are going to release it, we don't
+    // want the core to use it from this point. This won't happen as
+    // the media player must be stopped.
+    libvlc_media_player_set_nsobject(instance, nil, NULL);
+
+    libvlc_media_player_release(instance);
     
     // Get rid of everything else
     [media release];
     [cachedTime release];
+    [drawable release];
 
     [super dealloc];
 }
@@ -503,16 +511,6 @@ static void HandleMediaInstanceStateChanged(const libvlc_event_t * event, void *
 
 - (void)stop
 {
-    if( 0 && [NSThread isMainThread] )
-    {
-        /* Hack because we create a dead lock here, when the vout is stopped
-         * and tries to recontact us on the main thread */
-        /* FIXME: to do this properly we need to do some locking. We may want 
-         * to move that to libvlc */
-        [self performSelectorInBackground:@selector(stop) withObject:nil];
-        return;
-    }
-    
     libvlc_exception_t ex;
     libvlc_exception_init( &ex );
     libvlc_media_player_stop((libvlc_media_player_t *)instance, &ex);