Work-around window-on-top + fullscreen deadlock (refs #882)

FIXME FIXME FIXME FIXME: EXPLICIT HACK.
On the one hand, we cannot hold the lock while triggering a callback, as
it causes a deadlock with video-on-top handling. On the other hand, we
have to lock while triggering the callback to:
 1/ make sure video-on-top remains in sync with fullscreen (i.e.
    unlocking creates a race condition if fullscreen is switched on and
    off VERY FAST).
 2/ avoid possible corruption bugs if another thread gets the mutex and
    modifies our data in-between (though it does not seem like it could really
    do much harm in this particular case).

This is obviously contradictory. Correct solutions may include:
 - putting the fullscreen NAND video-on-top logic out of libvlc,
   back into the video output plugins (ugly code duplication...),
 - serializing fullscreen and video-on-top handling properly instead of
   using the fullscreen callback. That's got to be the correct one.

diff --git a/modules/video_output/x11/xcommon.c b/modules/video_output/x11/xcommon.c
index 0868742fd563f63a39f63bf1736220041fb0fa78..4644dbf8e515dd0954d69308a0d02f372fcfbc17 100644 (file)
--- a/modules/video_output/x11/xcommon.c
+++ b/modules/video_output/x11/xcommon.c
@@ -1326,8 +1326,38 @@ static int ManageVideo( vout_thread_t *p_vout )
 
         /* Update the object variable and trigger callback */
         val.b_bool = !p_vout->b_fullscreen;
+
+        /*
+         * FIXME FIXME FIXME FIXME: EXPLICIT HACK.
+         * On the one hand, we cannot hold the lock while triggering a
+         * callback, as it causes a deadlock with video-on-top handling.
+         * On the other hand, we have to lock while triggering the
+         * callback to:
+         *  1/ make sure video-on-top remains in sync with fullscreen
+         *    (i.e. unlocking creates a race condition if fullscreen is
+         *     switched on and off VERY FAST).
+         *  2/ avoid possible corruption bugs if another thread gets the
+         *     mutex and modifies our data in-between.
+         *
+         * This is obviously contradictory. Correct solutions may include:
+         *  - putting the fullscreen NAND video-on-top logic out of libvlc,
+         *    back into the video output plugins (ugly code duplication...),
+         *  - serializing fullscreen and video-on-top handling properly
+         *    instead of doing it via the fullscreen callback. That's got to
+         *    be the correct one.
+         */
+#ifdef MODULE_NAME_IS_xvmc
+        xvmc_context_reader_unlock( &p_vout->p_sys->xvmc_lock );
+#endif
+        vlc_mutex_unlock( &p_vout->p_sys->lock );
+
         var_Set( p_vout, "fullscreen", val );
 
+        vlc_mutex_lock( &p_vout->p_sys->lock );
+#ifdef MODULE_NAME_IS_xvmc
+        xvmc_context_reader_lock( &p_vout->p_sys->xvmc_lock );
+#endif
+
         ToggleFullScreen( p_vout );
         p_vout->i_changes &= ~VOUT_FULLSCREEN_CHANGE;
     }