On 01/30/2017 07:33 AM, Eli Zaretskii wrote:
For example, filelock.c's read_lock_data opens a file, uses emacs_read to read it, and then closes the file. If read_lock_data used emacs_read_quit it might process a quit, which would skip the close and leak a file descriptor. The read_lock_data issue could be fixed by having it call record_unwind_protect_int (close_file_unwind, fd) before calling emacs_read. Possibly all these dicey uses of emacs_read could be fixed in a similar way. However, that would be a bigger and more-intrusive change, and in the read_lock_data case it arguably would be overkill and I wanted to keep the patch smaller. I used emacs_read_quit only in places that I verified were safe, and stuck with emacs_read when I wasn't sure, or where more-intrusive changes would be needed.I don't understand why we need both this and emacs_read, which cannot be interrupted. Why not have just emacs_read which can be interrupted, and use that all over?
@@ -1252,6 +1256,7 @@ search_buffer (Lisp_Object string, ptrdiff_t pos,
ptrdiff_t pos_byte,
return (n);
}
n++;
+ maybe_quit ();
}
while (n > 0)
{
regex.c calls maybe_quit internally, so why do we need this additional
call?
@@ -724,6 +725,8 @@ back_comment (ptrdiff_t from, ptrdiff_t from_byte,
ptrdiff_t stop,
that determines quote parity to the comment-end. */
while (from != stop)
{
+ incr_rarely_quit (&quit_count);
+
Is it safe to quit from back_comment? It manipulates a global
variable gl_state, and I don't see unwind-protect calls anywhere in
sight.
@@ -10445,30 +10433,12 @@ handle_interrupt (bool in_signal_handler)
}
else
{
- /* If executing a function that wants to be interrupted out of
- and the user has not deferred quitting by binding `inhibit-quit'
- then quit right away. */
- if (immediate_quit && NILP (Vinhibit_quit))
- {
- struct gl_state_s saved;
-
- immediate_quit = false;
- pthread_sigmask (SIG_SETMASK, &empty_mask, 0);
- saved = gl_state;
- quit ();
- gl_state = saved;
- }
- else
- { /* Else request quit when it's safe. */
- int count = NILP (Vquit_flag) ? 1 : force_quit_count + 1;
- force_quit_count = count;
- if (count == 3)
- {
- immediate_quit = true;
- Vinhibit_quit = Qnil;
- }
- Vquit_flag = Qt;
- }
+ /* Request quit when it's safe. */
+ int count = NILP (Vquit_flag) ? 1 : force_quit_count + 1;
+ force_quit_count = count;
+ if (count == 3)
+ Vinhibit_quit = Qnil;
+ Vquit_flag = Qt;
}
This loses the feature whereby C-g on a TTY would quit much faster.
Why is this a good idea?
Inertia, I think. Having C-g generate SIGINT made sense when we had immediate_quit. I expect that it is a useless appendage now, and that in a later patch we can change Emacs so that C-g no longer generates SIGINT but is instead processed like any other input character.And if it is a good idea, why do we still generate SIGINT on C-g (and force GDB to handle SIGINT specially to support that)?
Attachment:
0001-Remove-immediate_quit.patch
Description: Source code patch
Attachment:
0002-Revamp-quitting-and-fix-infloops.patch
Description: Source code patch
Attachment:
0003-Fix-quitting-bug-when-buffers-are-frozen.patch
Description: Source code patch