# HG changeset patch # User Bram Moolenaar # Date 1565903704 -7200 # Node ID 10696f279e20e8a1c7e36b3a7cad4a963a8fa276 # Parent bf211be8ffd138789c8bc30a3a4dc7e91074d8ce patch 8.1.1851: crash when sound_playfile() callback plays sound commit https://github.com/vim/vim/commit/28e67e0c1496b7bb166a0acfb176690f219101ca Author: Bram Moolenaar Date: Thu Aug 15 23:05:49 2019 +0200 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 --- 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 --- 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 --- 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 --- a/src/os_unix.c +++ b/src/os_unix.c @@ -5998,6 +5998,11 @@ WaitForCharOrMouse(long msec, int *inter 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 --- 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 --- 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; + soundcb_T *soundcb = (soundcb_T *)userdata; + soundcb_queue_T *scb; - 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 + 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; +} + +/* + * Return TRUE if there is a sound callback to be called. + */ + int +has_sound_callback_in_queue(void) +{ + return callback_queue != NULL; +} - call_callback(&soundcb->snd_callback, -1, &rettv, 2, argv); - clear_tv(&rettv); +/* + * Invoke queued sound callbacks. + */ + void +invoke_sound_callback(void) +{ + soundcb_queue_T *scb; + typval_T argv[3]; + typval_T rettv; + - delete_sound_callback(soundcb); + while (callback_queue != NULL) + { + scb = callback_queue; + callback_queue = scb->scb_next; + + 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 --- 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 --- 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 - && (has_pending_job() || channel_any_readahead())) +# ifdef FEAT_SOUND_CANBERRA + has_any_sound_callback() +# endif + )) { // 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 --- 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,