[vlc-devel] commit: macosx/framework: Make sure the nsobject drawable is never used after being freed. (Pierre d 'Herbemont )

[git version control]

vlc | branch: master | Pierre d'Herbemont <pdherbemont at free.fr> | Mon Dec  7 18:49:02 2009 +0100| [2a45504451ead32bea132c360c6b0ba5470fd976] | committer: Pierre d'Herbemont 

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.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=2a45504451ead32bea132c360c6b0ba5470fd976
---

 modules/gui/minimal_macosx/VLCOpenGLVoutView.h     |    2 +-
 modules/gui/minimal_macosx/VLCOpenGLVoutView.m     |    7 ++++++-
 .../framework/Headers/Public/VLCMediaPlayer.h      |    1 +
 projects/macosx/framework/Sources/VLCMediaPlayer.m |   20 +++++++++-----------
 4 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/modules/gui/minimal_macosx/VLCOpenGLVoutView.h b/modules/gui/minimal_macosx/VLCOpenGLVoutView.h
index 406887a..3045843 100644
--- 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 */
- at protocol VLCOpenGLVoutEmbedding
+ at 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 8b50b42..fe6fbe3 100644
--- 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 0da9e46..6112f47 100644
--- 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 4954260..5909595 100644
--- 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);