patch 8.1.1851: crash when sound_playfile() callback plays sound

Problem:    Crash when sound_playfile() callback plays sound.
Solution:   Invoke callback later from event loop.

diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index ea58efa1d19858197ad6e9f0a733fc5463233b45..6369f10aa32328f91af719fde8561f0cac071dd9 100644 (file)
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -7692,8 +7692,12 @@ do_sleep(long msec)
        }
 #endif
 #ifdef FEAT_JOB_CHANNEL
-       if (has_any_channel() && wait_now > 100L)
-           wait_now = 100L;
+       if (has_any_channel() && wait_now > 20L)
+           wait_now = 20L;
+#endif
+#ifdef FEAT_SOUND
+       if (has_any_sound_callback() && wait_now > 20L)
+           wait_now = 20L;
 #endif
        ui_delay(wait_now, TRUE);
 #ifdef FEAT_JOB_CHANNEL

diff --git a/src/feature.h b/src/feature.h
index 89d6caef401083320adb6176acc48377b8df96f8..64689c96d7d7e416d31156c8f4a596edf4b7b792 100644 (file)
--- a/src/feature.h
+++ b/src/feature.h
@@ -666,6 +666,9 @@
 #if !defined(FEAT_SOUND) && defined(HAVE_CANBERRA)
 # define FEAT_SOUND
 #endif
+#if defined(FEAT_SOUND) && defined(HAVE_CANBERRA)
+# define FEAT_SOUND_CANBERRA
+#endif
 
 /* There are two ways to use XPM. */
 #if (defined(HAVE_XM_XPMP_H) && defined(FEAT_GUI_MOTIF)) \

diff --git a/src/misc2.c b/src/misc2.c
index eb6cf554338e0fb09b6ce7ffeb1984df6cdc6a70..463b37b79d40ee575ecfc585f980c0642730d19b 100644 (file)
--- a/src/misc2.c
+++ b/src/misc2.c
@@ -4485,6 +4485,10 @@ parse_queued_messages(void)
 # endif
 # ifdef FEAT_TERMINAL
        free_unused_terminals();
+# endif
+# ifdef FEAT_SOUND_CANBERRA
+       if (has_sound_callback_in_queue())
+           invoke_sound_callback();
 # endif
        break;
     }

diff --git a/src/os_unix.c b/src/os_unix.c
index 6387a7757a0c5286944628c4b4c742d3f68d6b37..d80fb1db034c1deb89a905e47b233b34fca2c53e 100644 (file)
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -5998,6 +5998,11 @@ WaitForCharOrMouse(long msec, int *interrupted, int ignore_input)
                rest -= msec;
        }
 # endif
+# ifdef FEAT_SOUND_CANBERRA
+       // Invoke any pending sound callbacks.
+       if (has_sound_callback_in_queue())
+           invoke_sound_callback();
+# endif
 # ifdef FEAT_MOUSE_GPM
        gpm_process_wanted = 0;
        avail = RealWaitForChar(read_cmd_fd, msec,

diff --git a/src/proto/sound.pro b/src/proto/sound.pro
index cf8439ad43a2eb5f87298853f2fe754b35e9b54d..ad1c2b5305399f6137f865907aa8290362abf333 100644 (file)
--- a/src/proto/sound.pro
+++ b/src/proto/sound.pro
@@ -1,4 +1,7 @@
 /* sound.c */
+int has_any_sound_callback(void);
+int has_sound_callback_in_queue(void);
+void invoke_sound_callback(void);
 void f_sound_playevent(typval_T *argvars, typval_T *rettv);
 void f_sound_playfile(typval_T *argvars, typval_T *rettv);
 void f_sound_stop(typval_T *argvars, typval_T *rettv);

diff --git a/src/sound.c b/src/sound.c
index 92618a9f341a65e6a5c989a8fd8153074c2b28d8..69bbc5e6d91f48d7d916fbf141ab53ad5b218bdd 100644 (file)
--- a/src/sound.c
+++ b/src/sound.c
@@ -30,6 +30,16 @@ struct soundcb_S {
 
 static soundcb_T    *first_callback = NULL;
 
+/*
+ * Return TRUE when a sound callback has been created, it may be invoked when
+ * the sound finishes playing.  Also see has_sound_callback_in_queue().
+ */
+    int
+has_any_sound_callback(void)
+{
+    return first_callback != NULL;
+}
+
     static soundcb_T *
 get_sound_callback(typval_T *arg)
 {
@@ -85,6 +95,24 @@ delete_sound_callback(soundcb_T *soundcb)
 
 static ca_context   *context = NULL;
 
+// Structure to store info about a sound callback to be invoked soon.
+typedef struct soundcb_queue_S soundcb_queue_T;
+
+struct soundcb_queue_S {
+    soundcb_queue_T    *scb_next;
+    uint32_t           scb_id;         // ID of the sound
+    int                        scb_result;     // CA_ value
+    soundcb_T          *scb_callback;  // function to call
+};
+
+// Queue of callbacks to invoke from the main loop.
+static soundcb_queue_T *callback_queue = NULL;
+
+/*
+ * Add a callback to the queue of callbacks to invoke later from the main loop.
+ * That is because the callback may be called from another thread and invoking
+ * another sound function may cause trouble.
+ */
     static void
 sound_callback(
        ca_context  *c UNUSED,
@@ -92,23 +120,58 @@ sound_callback(
        int         error_code,
        void        *userdata)
 {
-    soundcb_T  *soundcb = (soundcb_T *)userdata;
-    typval_T   argv[3];
-    typval_T   rettv;
-
-    argv[0].v_type = VAR_NUMBER;
-    argv[0].vval.v_number = id;
-    argv[1].v_type = VAR_NUMBER;
-    argv[1].vval.v_number = error_code == CA_SUCCESS ? 0
+    soundcb_T      *soundcb = (soundcb_T *)userdata;
+    soundcb_queue_T *scb;
+
+    scb = ALLOC_ONE(soundcb_queue_T);
+    if (scb == NULL)
+       return;
+    scb->scb_next = callback_queue;
+    callback_queue = scb;
+    scb->scb_id = id;
+    scb->scb_result = error_code == CA_SUCCESS ? 0
                          : error_code == CA_ERROR_CANCELED
                                            || error_code == CA_ERROR_DESTROYED
                          ? 1 : 2;
-    argv[2].v_type = VAR_UNKNOWN;
+    scb->scb_callback = soundcb;
+}
 
-    call_callback(&soundcb->snd_callback, -1, &rettv, 2, argv);
-    clear_tv(&rettv);
+/*
+ * Return TRUE if there is a sound callback to be called.
+ */
+    int
+has_sound_callback_in_queue(void)
+{
+    return callback_queue != NULL;
+}
+
+/*
+ * Invoke queued sound callbacks.
+ */
+    void
+invoke_sound_callback(void)
+{
+    soundcb_queue_T *scb;
+    typval_T       argv[3];
+    typval_T       rettv;
+
+
+    while (callback_queue != NULL)
+    {
+       scb = callback_queue;
+       callback_queue = scb->scb_next;
 
-    delete_sound_callback(soundcb);
+       argv[0].v_type = VAR_NUMBER;
+       argv[0].vval.v_number = scb->scb_id;
+       argv[1].v_type = VAR_NUMBER;
+       argv[1].vval.v_number = scb->scb_result;
+       argv[2].v_type = VAR_UNKNOWN;
+
+       call_callback(&scb->scb_callback->snd_callback, -1, &rettv, 2, argv);
+       clear_tv(&rettv);
+
+       delete_sound_callback(scb->scb_callback);
+    }
     redraw_after_callback(TRUE);
 }
 

diff --git a/src/testdir/test_sound.vim b/src/testdir/test_sound.vim
index e74aa132da8542b52f01386234f8d8010281f045..6ffd7e65f614a207bec53f9d10f9ae8b0943f4c9 100644 (file)
--- a/src/testdir/test_sound.vim
+++ b/src/testdir/test_sound.vim
@@ -13,7 +13,6 @@ func Test_play_event()
   if has('win32')
     throw 'Skipped: Playing event with callback is not supported on Windows'
   endif
-
   let id = sound_playevent('bell', 'PlayCallback')
   if id == 0
     throw 'Skipped: bell event not available'
@@ -21,7 +20,7 @@ func Test_play_event()
   " Stop it quickly, avoid annoying the user.
   sleep 20m
   call sound_stop(id)
-  sleep 20m
+  sleep 30m
   call assert_equal(id, g:id)
   call assert_equal(1, g:result)  " sound was aborted
 endfunc
@@ -46,6 +45,20 @@ func Test_play_silent()
   call assert_true(id2 > 0)
   sleep 20m
   call sound_clear()
+  sleep 30m
   call assert_equal(id2, g:id)
   call assert_equal(1, g:result)
+
+  " recursive use was causing a crash
+  func PlayAgain(id, fname)
+    let g:id = a:id
+    let g:id_again = sound_playfile(a:fname)
+  endfunc
+
+  let id3 = sound_playfile(fname, {id, res -> PlayAgain(id, fname)})
+  call assert_true(id3 > 0)
+  sleep 50m
+  call sound_clear()
+  sleep 30m
+  call assert_true(g:id_again > 0)
 endfunc

diff --git a/src/ui.c b/src/ui.c
index c8cedb95550757ddc15c829203ed077de974b706..1b1b2cd645bd0da2981bade9504d09bcbcf12668 100644 (file)
--- a/src/ui.c
+++ b/src/ui.c
@@ -459,12 +459,22 @@ ui_wait_for_chars_or_timer(
        }
        if (due_time <= 0 || (wtime > 0 && due_time > remaining))
            due_time = remaining;
-# ifdef FEAT_JOB_CHANNEL
-       if ((due_time < 0 || due_time > 10L)
-#  ifdef FEAT_GUI
-               && !gui.in_use
+# if defined(FEAT_JOB_CHANNEL) || defined(FEAT_SOUND_CANBERRA)
+       if ((due_time < 0 || due_time > 10L) && (
+#  if defined(FEAT_JOB_CHANNEL)
+               (
+#   if defined(FEAT_GUI)
+               !gui.in_use &&
+#   endif
+               (has_pending_job() || channel_any_readahead()))
+#   ifdef FEAT_SOUND_CANBERRA
+               ||
+#   endif
+#  endif
+#  ifdef FEAT_SOUND_CANBERRA
+                   has_any_sound_callback()
 #  endif
-               && (has_pending_job() || channel_any_readahead()))
+                   ))
        {
            // There is a pending job or channel, should return soon in order
            // to handle them ASAP.  Do check for input briefly.

diff --git a/src/version.c b/src/version.c
index 64a43beb6e17972f44742fb3a5d242bd4100d30d..93867b87dca85f0de6c0ac9fd66c6ec6a0f616f1 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -769,6 +769,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1851,
 /**/
     1850,
 /**/