GNU bug report logs -
#76969
kill-buffer fails silently when a thread exists where it's current
Previous Next
Full log
Message #71 received at 76969 <at> debbugs.gnu.org (full text, mbox):
On 17/07/2025 09:25, Eli Zaretskii wrote:
>> 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)
Do you think there is a chance for the thread to be started between the
'make-thread' and 'thread-set-buffer-killable' calls, even if they go
one after another?
I considered making it a keyword argument of 'make-thread' but it seems
like it might not interest most callers, and so clutter the definition
(I couldn't come up with a less awkward name either). Not a strong
opinion, though.
> . 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)
Sounds good to me. Guess that calls for removing '-p' from the name too.
> . we need documentation changes; in particular some of these changes
> should be called out as incompatible
Sure.
>> + 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.
Will update in the next revision. It will probably complicate the
calling convention - and create another condition for keeping the buffer.
>> + // 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.
I'll probably keep this line as-is for now, and we could revisit this as
a small optimization later.
> And note that doing that could potentially switch threads, which we
> perhaps want to avoid here?
Are you referring to the 'tstate->wait_condvar' check? Hmm, I suppose it
could hit if the current thread first releases a mutex held by another
thread, and then kills its buffer.
If that's realistic, a helper function that doesn't do that, might
indeed be better.
>> @@ -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?
I didn't want to decide on this myself. Could certainly prohibit it,
looking from safety's perspective.
Unless somebody thinks of a scenario that would make it useful?
>> + 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?
Yes, thanks.
This bug report was last modified 4 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.