patch 8.1.1851: crash when sound_playfile() callback plays soundv8.1.1851

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

9 files changed, 128 insertions, 21 deletions

diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index ea58efa1d..6369f10aa 100644
--- 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 89d6caef4..64689c96d 100644
--- 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 eb6cf5543..463b37b79 100644
--- a/src/misc2.c
+++ b/src/misc2.c
@@ -4486,6 +4486,10 @@ parse_queued_messages(void)
 # 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 6387a7757..d80fb1db0 100644
--- 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 cf8439ad4..ad1c2b530 100644
--- 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 92618a9f3..69bbc5e6d 100644
--- 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 e74aa132d..6ffd7e65f 100644
--- 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 c8cedb955..1b1b2cd64 100644
--- 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 64a43beb6..93867b87d 100644
--- a/src/version.c
+++ b/src/version.c
@@ -770,6 +770,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    1851,
+/**/
     1850,
 /**/
     1849,