GNU bug report logs - #76969
kill-buffer fails silently when a thread exists where it's current

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: sbaugh <at> janestreet.com, 76969 <at> debbugs.gnu.org
Subject: bug#76969: kill-buffer fails silently when a thread exists where it's current
Date: Thu, 17 Jul 2025 09:25:45 +0300

> Date: Sun, 13 Jul 2025 17:02:07 +0300
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> Cc: 76969 <at> debbugs.gnu.org
> 
> Here's a patch with implementation:
> 
> * It's a property of the thread, on by default.
> * Set to nil for the main thread (currently can be changed from Lisp).
> * Not a buffer-local variable - and looking more at it, that seems not a 
> good idea, because several threads could have the same buffer current, 
> and some might be okay with this behavior, while others not.
> * A couple of questions about the finer details of the implementation, 
> see comments in 'thread_all_before_buffer_killed'.
> * Two tests included.
> 
> Seems to work well in my testing, and in the diff-hl scenario described 
> in the report: step 5 kills the buffer and sends the signal which I can 
> catch inside condition-case in diff-hl--update-safe.
> 
> WDYT?

It's a starting point, thanks.  Comments:

 . I think we should add an optional argument to make-thread, because
   calling a function to change the buffer_killable_p attribute might
   be too late (I'm not against having the function as well)
 . I think we should also allow a thread to say it's fine to kill the
   current buffer, but not to signal the thread when the buffer is
   killed (this means the buffer_killable_p should be not just a
   simple boolean)
 . we need documentation changes; in particular some of these changes
   should be called out as incompatible

> +  for (iter = all_threads; iter; iter = iter->next_thread)
> +    {
> +      if (iter == caller_thread)
> +	continue;
> +
> +      /* Could iter->buffer_killable_p be false now?
> +	 E.g. changed by hooks.  */

It is best not to rely on it being true, yes.

> +	  // Or we could inline the last part of its definition.
> +	  Fthread_signal (thread, Qthread_buffer_killed, Qnil);

It is better to make that last part be a function, and call it in both
places.

And note that doing that could potentially switch threads, which we
perhaps want to avoid here?

> @@ -1174,6 +1247,7 @@ init_threads (void)
>  #endif /* defined HAVE_ANDROID && !defined ANDROID_STUBIFY */
>  
>    main_thread.s.thread_id = sys_thread_self ();
> +  main_thread.s.buffer_killable_p = false;

Do we allow calling thread-set-buffer-killable on the main thread?

> +  DEFSYM (Qthread_buffer_killed, "thread-buffer-killed");
> +  Fput (Qthread_buffer_killed, Qerror_conditions,
> +	list (Qthread_buffer_killed, Qerror));
> +  Fput (Qscan_error, Qerror_message,
           ^^^^^^^^^^^
Copy/pasta?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.