Re: [PATCH v3 0/6] Make Emacs search use sexp format
authorJameson Graef Rollins <jrollins@finestructure.net>
Wed, 5 Jun 2013 15:21:59 +0000 (08:21 +1700)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:55:26 +0000 (09:55 -0800)
6d/1c016b50eb94b67dc2a17c7fb842dbf39fffa0 [new file with mode: 0644]

diff --git a/6d/1c016b50eb94b67dc2a17c7fb842dbf39fffa0 b/6d/1c016b50eb94b67dc2a17c7fb842dbf39fffa0
new file mode 100644 (file)
index 0000000..ab7b59e
--- /dev/null
@@ -0,0 +1,208 @@
+Return-Path: <jrollins@finestructure.net>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id 15171431FAF\r
+       for <notmuch@notmuchmail.org>; Wed,  5 Jun 2013 08:22:23 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -2.3\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5\r
+       tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id l69V0CyDdLoY for <notmuch@notmuchmail.org>;\r
+       Wed,  5 Jun 2013 08:22:15 -0700 (PDT)\r
+Received: from outgoing-mail.its.caltech.edu (outgoing-mail.its.caltech.edu\r
+       [131.215.239.19])\r
+       by olra.theworths.org (Postfix) with ESMTP id B4DC2431FAE\r
+       for <notmuch@notmuchmail.org>; Wed,  5 Jun 2013 08:22:15 -0700 (PDT)\r
+Received: from fire-doxen.imss.caltech.edu (localhost [127.0.0.1])\r
+       by fire-doxen-postvirus (Postfix) with ESMTP id 2C958328004;\r
+       Wed,  5 Jun 2013 08:22:13 -0700 (PDT)\r
+X-Spam-Scanned: at Caltech-IMSS on fire-doxen by amavisd-new\r
+Received: from finestructure.net (cpe-76-173-75-86.socal.res.rr.com\r
+       [76.173.75.86]) (Authenticated sender: jrollins)\r
+       by fire-doxen-submit (Postfix) with ESMTP id 831E4328025;\r
+       Wed,  5 Jun 2013 08:22:03 -0700 (PDT)\r
+Received: by finestructure.net (Postfix, from userid 1000)\r
+       id CB1906171A; Wed,  5 Jun 2013 08:22:02 -0700 (PDT)\r
+From: Jameson Graef Rollins <jrollins@finestructure.net>\r
+To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v3 0/6] Make Emacs search use sexp format\r
+In-Reply-To: <87bo7mtp79.fsf@awakening.csail.mit.edu>\r
+References: <1370047208-12785-1-git-send-email-amdragon@mit.edu>\r
+       <87sj12yqyu.fsf@maritornes.cs.unb.ca>\r
+       <87r4gk8qa5.fsf@servo.finestructure.net>\r
+       <87bo7mtp79.fsf@awakening.csail.mit.edu>\r
+User-Agent: Notmuch/0.15.2+155~g7fa0560 (http://notmuchmail.org) Emacs/24.3.1\r
+       (x86_64-pc-linux-gnu)\r
+Date: Wed, 05 Jun 2013 08:21:59 -0700\r
+Message-ID: <87a9n460rs.fsf@servo.finestructure.net>\r
+MIME-Version: 1.0\r
+Content-Type: multipart/signed; boundary="=-=-=";\r
+       micalg=pgp-sha256; protocol="application/pgp-signature"\r
+Cc: tomi.ollila@iki.fi\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Wed, 05 Jun 2013 15:22:23 -0000\r
+\r
+--=-=-=\r
+Content-Type: text/plain\r
+\r
+On Mon, Jun 03 2013, Austin Clements <amdragon@MIT.EDU> wrote:\r
+>> * Killing a search buffer that is still in the process of being filled\r
+>>   causes errors to be thrown.  I'm seeing both of the following\r
+>>   intermittently:\r
+>>\r
+>> [Sun Jun  2 08:26:40 2013]\r
+>> notmuch exited with status killed\r
+>> command: notmuch search --format\=sexp --format-version\=1 --sort\=newest-first to\:jrollins\r
+>> exit signal: killed\r
+>>\r
+>> [Sun Jun  2 08:32:26 2013]\r
+>> notmuch exited with status hangup\r
+>> command: notmuch search --format\=sexp --format-version\=1 --sort\=newest-first to\:jrollins\r
+>> exit signal: hangup\r
+>>\r
+>>   This is somewhat understandable, as the notmuch binary exits with an\r
+>>   error if it hasn't finished dumping the output, but given how common\r
+>>   this particular scenario is I think we should try to avoid throwing\r
+>>   errors in this circumstance.  I wonder if we shouldn't just modify the\r
+>>   binary to not return non-zero if it was manually killed while\r
+>>   processing the output, or at least special-case the particular error\r
+>>   caused by manually killing the search.\r
+>\r
+> Your assessment is correct, of course.  The right place to fix this is\r
+> in Emacs, not the CLI (the CLI *can't* do anything about this, since it\r
+> gets killed by a signal).  Probably we should do something different in\r
+> the sentinel if the search process's buffer is no longer live.  Clearly\r
+> we should suppress the status error for the signal, but I think we still\r
+> should report anything that appeared in err-file because it may be\r
+> relevant to why the user killed the buffer (e.g., maybe a notmuch\r
+> wrapper was blocked on something).\r
+\r
+That seems like a reasonable approach to me, to suppress the error but\r
+continue to report in *Notmuch errors* buffer.\r
+\r
+>> * The next thing I'm seeing is this:\r
+>>\r
+>> Opening input file: no such file or directory, /home/jrollins/tmp/nmerr5390CAY\r
+>>\r
+>>   I'm not exactly sure what causes this error, but it looks to me like\r
+>>   the temporary error file was removed before we were finished with it.\r
+>\r
+> This one's pretty awesome (and I think is a bug in Emacs).  At a high\r
+> level, the sentinel is getting run twice and since the first call\r
+> deletes the error file, the second call fails.  At a low level, what\r
+> causes this is fascinating.\r
+>\r
+> 1) You kill the search buffer.  This invokes kill_buffer_processes,\r
+>    which sends a SIGHUP to notmuch, but doesn't do anything else.\r
+>    Meanwhile, the notmuch search process has printed some more output,\r
+>    but Emacs hasn't consumed it yet (this is critical).\r
+>\r
+> 2) Emacs gets a SIGCHLD from the dying notmuch process, which invokes\r
+>    handle_child_signal, which sets the new process status, but can't do\r
+>    anything else because it's a signal handler.\r
+>\r
+> 3) Emacs returns to its idle loop, which calls status_notify, which sees\r
+>    that the notmuch process has a new status.  This is where things get\r
+>    interesting.\r
+>\r
+> 3.1) Emacs guarantees that it will run process filters on any unconsumed\r
+>      output before running the process sentinel, so status_notify calls\r
+>      read_process_output, which consumes the final output and calls\r
+>      notmuch-search-process-filter.\r
+>\r
+> 3.1.1) notmuch-search-process-filter contains code to check if the\r
+>        search buffer is still alive and, since it's not, it calls\r
+>        delete-process.\r
+>\r
+> 3.1.1.1) delete-process correctly sees that the process is already dead\r
+>          and doesn't try to send another signal, *but* it still modifies\r
+>          the status to "killed".  To deal with the new status, it calls\r
+>          status_notify.  Dun dun dun.  We've seen this function before.\r
+>\r
+> 3.1.1.1.1) The *recursive* status_notify invocation sees that the\r
+>            process has a new status and doesn't have any more output to\r
+>            consume, so it invokes our sentinel and returns.\r
+>\r
+> 3.2) The outer status_notify call (which we're still in) is now done\r
+>      flushing pending process output, so it *also* invokes our sentinel.\r
+>\r
+> It might be that the answer is to just remove the delete-process call\r
+> from the filter.  It seems completely redundant (and racy) with Emacs'\r
+> automatic SIGHUP'ing.\r
+\r
+Wow, awesome detective work.  As mentioned on IRC, this suggestion of\r
+Austin's does seem to fix the problem:\r
+\r
+diff --git a/emacs/notmuch.el b/emacs/notmuch.el\r
+index 5a8c957..975ef2b 100644\r
+--- a/emacs/notmuch.el\r
++++ b/emacs/notmuch.el\r
+@@ -817,7 +817,7 @@ non-authors is found, assume that all of the authors match."\r
+        (inhibit-read-only t)\r
+        done)\r
+     (if (not (buffer-live-p results-buf))\r
+-       (delete-process proc)\r
++       t\r
+       (with-current-buffer parse-buf\r
+        ;; Insert new data\r
+        (save-excursion\r
+\r
+\r
+I'm not sure if this is the ultimate solution, but it does cause the\r
+missing tmp file errors to go away.\r
+\r
+>> * Finally, something happened that caused *12,000* of the following lines\r
+>>   to be sent to the *Notmuch errors* buffer:\r
+>>\r
+>> A Xapian exception occurred performing query: The revision being read has been discarded - you should call Xapian::Database::reopen() and retry the operation\r
+>>\r
+>>   Again, this was related to killing a search buffer that was still\r
+>>   being filled. I'm pretty sure the database was not modified during\r
+>>   this process.\r
+>\r
+> I have no insight on this one.  My best guess is that this has nothing\r
+> to do with this change except that this change makes these warnings\r
+> visible rather than burying them somewhere down in the search results\r
+> buffer.\r
+\r
+Yeah, I suspected as much as well.\r
+\r
+jamie.\r
+\r
+--=-=-=\r
+Content-Type: application/pgp-signature\r
+\r
+-----BEGIN PGP SIGNATURE-----\r
+Version: GnuPG v1.4.12 (GNU/Linux)\r
+\r
+iQIcBAEBCAAGBQJRr1eYAAoJEO00zqvie6q8d4IP/RWe/4CnDqQSm9QQLSoUfNwm\r
+tIuDzsvo4reBrNlPwQrB5+jKtVTcd7byDv/OJcTbyr44M3qy/2LUzQOexm1WBGj1\r
+wyg/qaiAKac0KjY/3zaxGwkXe/CMgfKJ3/zcBOnUFk3TadRs+KiKdZM6aS2KgQj/\r
+OjSTZoVd9/6MTDf3je5fwcl+J74I0rOYmntK37PuRcMrNmSOsFxMH8sAvp8KgkFF\r
+jY/IJ3mq/sS7/Juc02HN5IWKlByU6t7W3IqoYIbLfwT/TlyFZgMxXBHiae3nFYyx\r
+WnJaYzU0zz7bFem0eka1uwhEQuvECeYBZmo5FrHF20uzPNpeMf2SH7P3hA39TQhn\r
+E5HiKQzBNj5N67+7t8xrnaAbYPv+kTRO1iy7HjxjYoG0XcW9iJkz3lyqi9cqsLHu\r
+gbA+VaiUBoEws3afDj3AJ+scDfW6pRgSWVi+nfMDaqziRKn3rKSquNjlXuWnW6AA\r
+jjkL9P4OF3LpNJzilt1j+ocfqtmI86dj/Z8xK92Meh4zCBSMmq3DmXtkIm7uLQ8k\r
+rOE4BCCNa1uuMBVP8x1iz3ogHg4Nygsjh8N8tgww+a50rWrXTv5FLAUDpwCTI7Fm\r
+075Qgm7SiU06R4bF2YcOze/qJBg/sZyirm7ZfaYS7SiYZO1PP9vT4VIcxNviIExc\r
+1WVjDwXoTkarE3EU6ika\r
+=1AsS\r
+-----END PGP SIGNATURE-----\r
+--=-=-=--\r