Subject: bug#76969: kill-buffer fails silently when a thread exists where it's current

From: Eli Zaretskii <[email-masked]>
Date: Fri, 18 Jul 2025 09:24:34 +0300

> Date: Fri, 18 Jul 2025 03:23:43 +0300
> Cc: [email-masked], [email-masked]
> From: Dmitry Gutov <[email-masked]>
> 
> >   . 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?

No.  But it makes no sense to provide a separate interface if it must
always be called immediately after make-thread, or else should be
expected to be unreliable.

> 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.

There's no need for keyword arguments.  make-thread has just 2
arguments as of now; adding one more is not going to make the calls
awkward, especially if most callers will never pass that additional
argument.

> >   . 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.

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.
> 
> 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?

The call to flush_stack_call_func if the test succeeds, yes.

> 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.

Yes.

> >> +  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?

It's a significant change in behavior, so I doubt that.