From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25547 invoked by alias); 2 Oct 2016 23:35:43 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 39543 Received: (qmail 17509 invoked from network); 2 Oct 2016 23:35:43 -0000 X-Qmail-Scanner-Diagnostics: from mail-pa0-f67.google.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(209.85.220.67):SA:0(0.0/5.0):. Processed in 0.771576 secs); 02 Oct 2016 23:35:43 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: schaefer@brasslantern.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at brasslantern.com does not designate permitted sender hosts) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=brasslantern-com.20150623.gappssmtp.com; s=20150623; h=from:message-id:date:in-reply-to:comments:references:to:subject :mime-version; bh=AtIrgxaFwiCHd+u25J+OFZJP027TJ6lmXIV1HgTsogk=; b=wmSJl+3watM205YmG+oE4kp6n2/JD9yBRytAouWqsrPKeROj78nrL4L6A49U0pDAJr 5yNTfYB7drxU8eX/Jd4y4+Oupdu4kxOo1P7oMSlGfnWZ7d0s5gxRkx/pX6Pz7UZJCaqC K77z7R7YJmCUONTOEWxtOzK+sgopgiMJ2jk8K+q03r/YqfxYc5bI+m01baYSPfW8B1F3 fy1WQVHs8AzXJdKfVz/LkufhTaz0ivdozeemfzhSkRPxZV23lccKVxKdy5dzcHVxsvtv FJ29fnWxjNS5lwVM8cuiNgVhW8DAa1psh+IIlktRScKZYfaSLJXrUtq/UKLJZ/K44e8c Xm6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:message-id:date:in-reply-to:comments :references:to:subject:mime-version; bh=AtIrgxaFwiCHd+u25J+OFZJP027TJ6lmXIV1HgTsogk=; b=QwK/v+f8CHkDULvPZl/vEaJ+S+aj8V+oUvLbkED5yOgx8nOOtYq1EHnHDh2FrRmGLt goGjn9zNVOueezcFWqu9tYnZ64EYSuvQAlUjQf6TmXJCD+5Pd3thnnHJtp4UCw8SzZr+ lK8Ci+NrFIDqMEVs8zwCQ1nmbzO5Vwk96cdexJ5tJpH4i6XWsd1Ec768laVmN7vkE8G9 qPDZD4n6RQKxiJz2vHaFJuXSMDIL7M1I9KezuPGUIxtLk96tRnLJgctW8lizIOiO8o32 CIjnctowhy8DXzb/cCEGUD1J9fi437OXZiO2IHVTGGV/zahcraET2a1wZoiWKqo4xyhn HtCQ== X-Gm-Message-State: AA6/9RkkPI4bVNqxl7P5oB3n+E1ZKWrkXCn7Z1vzcz8M5535izQCc0QKPBXJmO8DQJWdNQ== X-Received: by 10.66.142.9 with SMTP id rs9mr9629251pab.87.1475450482476; Sun, 02 Oct 2016 16:21:22 -0700 (PDT) From: Bart Schaefer Message-Id: <161002162145.ZM22574@torch.brasslantern.com> Date: Sun, 2 Oct 2016 16:21:45 -0700 In-Reply-To: Comments: In reply to Sebastian Gniazdowski "[BUG] queueing_enabled grows infinitely when in .recursive-edit" (Oct 2, 9:00pm) References: X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: Zsh hackers list Subject: Re: [BUG] queueing_enabled grows infinitely when in .recursive-edit MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii On Oct 2, 9:00pm, Sebastian Gniazdowski wrote: } } ... } } This causes Ctrl-C signal to be queued when in .recursive-edit, what } results in need of multiple Ctrl-C presses (errflag is set in middle } of sequence of checks of it's value and raw_getbytes() executes in } weird way). Also, errflag is set at random place after select() in } raw_getbyte(), and as Bart says, this prevents scheduled function to } execute at all, thus chain of rescheduling breaks. There seem to be a bunch of inter-related things going on here. The first is that recursiveedit() calls zlecore() which calls getkeycmd() which cascades into raw_getbyte() with do_keytmout = 0 which in some circumstances means that raw_getbyte() effectively does a blocking read on its first call and only runs the sched after a key is pressed. I haven't figured out what's wrong with the calc_timeout() logic that makes this possible, but it's a race -- it happens only once in a while. The second is that zlecore() expects to be entered with the signal queue disabled, but zle widgets are called with queueing enabled, so recursiveedit() needs save/zero/restore the queue level around the call to zlecore(). The third is that somewhere below execstring() from checksched(), the signal queueing level is being incremented but not decremented. The problem is that the execution code calls itself recursively so deeply that I can't pinpoint the place this occurs. Even with a watchpoint on queueing_enabled, it looks as though we should be fine -- it's as if the recursive calls never quite unwind all the way back to the top, but printing stack traces in gdb doesn't show that happening at any place where queueing_enabled changes. Usually the third effect is hidden by the restore_queue_signals() in getbyte(), but when in recursiveedit() the queue_signal_level() in raw_getbyte() starts out > 0, and never goes all the way back down again. Or something like that. I expected fixing the second problem to again mask the third, but it does not. It's almost like there's a tail-call optimization occuring that is causing an unqueue_signals() to be skipped. And in fact there are cases where the queueing_enabled is decremented but the watchpoint mysteriously does not trigger -- I see the smaller "old" value the next time the watchpoint triggers on the *increment*, but I never see the assignment that reduces the value. I think this is because gdb actually stops on the NEXT instruction AFTER the watched location, and there are some places where there is no breakable next line (the unqueue_signals() is the last line of a function). It's quite normal for queueing_enabled to run up to 12 or so on a normal execution stack and still decrement all the way back to zero, so watching for a high-water mark is nearly useless. Here's a patch (not to be committed) that causes a DPUTS() when the condition is detected, but that doesn't help with tracking down what causes it in the first place. diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c index 0bdd82b..ba7ef90 100644 --- a/Src/Zle/zle_main.c +++ b/Src/Zle/zle_main.c @@ -1867,11 +1867,17 @@ int recursiveedit(UNUSED(char **args)) { int locerror; + int q = queue_signal_level(); + + /* zlecore() expects to be entered with signal queue disabled */ + dont_queue_signals(); redrawhook(); zrefresh(); zlecore(); + restore_queue_signals(q); + locerror = errflag ? 1 : 0; errflag = done = eofsent = 0; diff --git a/Src/signals.c b/Src/signals.c index e2587dc..9b22dcd 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -67,7 +67,7 @@ static int exit_trap_posix; /* Variables used by signal queueing */ /**/ -mod_export int queueing_enabled, queue_front, queue_rear; +mod_export int queueing_enabled, queue_front, queue_rear, queue_in; /**/ mod_export int signal_queue[MAX_QUEUE_SIZE]; /**/ diff --git a/Src/signals.h b/Src/signals.h index d680968..cb6a171 100644 --- a/Src/signals.h +++ b/Src/signals.h @@ -82,7 +82,7 @@ #define MAX_QUEUE_SIZE 128 -#define queue_signals() (queueing_enabled++) +#define queue_signals() (queue_in++, queueing_enabled++) #define run_queued_signals() do { \ while (queue_front != queue_rear) { /* while signals in queue */ \ @@ -96,17 +96,23 @@ #define unqueue_signals() do { \ DPUTS(!queueing_enabled, "BUG: unqueue_signals called but not queueing"); \ + --queue_in; \ if (!--queueing_enabled) run_queued_signals(); \ } while (0) #define queue_signal_level() queueing_enabled #define dont_queue_signals() do { \ + queue_in = queueing_enabled; \ queueing_enabled = 0; \ run_queued_signals(); \ } while (0) -#define restore_queue_signals(q) (queueing_enabled = (q)) +#define restore_queue_signals(q) do { \ + DPUTS2(queueing_enabled && queue_in != q, \ + "BUG: q = %d != queue_in = %d", q, queue_in); \ + queue_in = (queueing_enabled = (q)); \ +} while (0) #ifdef BSD_SIGNALS #define signal_block(S) sigblock(S)