From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 55EA8421192 for ; Thu, 30 Jun 2011 12:38:28 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id TQCRSRbkckOB for ; Thu, 30 Jun 2011 12:38:27 -0700 (PDT) Received: from mail-ww0-f45.google.com (mail-ww0-f45.google.com [74.125.82.45]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id E213A42118E for ; Thu, 30 Jun 2011 12:38:26 -0700 (PDT) Received: by wwj40 with SMTP id 40so2109832wwj.2 for ; Thu, 30 Jun 2011 12:38:25 -0700 (PDT) Received: by 10.227.55.130 with SMTP id u2mr2226777wbg.41.1309462705402; Thu, 30 Jun 2011 12:38:25 -0700 (PDT) Received: from localhost ([109.131.21.173]) by mx.google.com with ESMTPS id ei4sm1850193wbb.43.2011.06.30.12.38.23 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 30 Jun 2011 12:38:24 -0700 (PDT) From: Pieter Praet To: Carl Worth , Mark Anderson , Robin Green Subject: Re: [PATCH 2/2] [RFC] possible solution for "Race condition for '*' command" In-Reply-To: <1309450108-2793-2-git-send-email-pieter@praet.org> References: <8739itagad.fsf@yoom.home.cworth.org> <1309450108-2793-1-git-send-email-pieter@praet.org> <1309450108-2793-2-git-send-email-pieter@praet.org> User-Agent: Notmuch/0.5-315-g34bd5eb (http://notmuchmail.org) Emacs/23.1.50.1 (x86_64-pc-linux-gnu) Date: Thu, 30 Jun 2011 21:38:22 +0200 Message-ID: <87r56brtxt.fsf@praet.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: notmuch@notmuchmail.org X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Jun 2011 19:38:28 -0000 On Thu, 30 Jun 2011 18:08:28 +0200, Pieter Praet wrote: > `notmuch-search-operate-all' may cause a race condition because > repeatedly running `notmuch-tag' with the *original* query string > makes the result list a moving target. > > One approach to resolving this, is to feed `notmuch-tag' a static result > list based on the original query string, instead of the latter itself. > > See discussion @ id:"86d3i1d06r.fsf@dragonfly.greenrd.org" > > Signed-off-by: Pieter Praet > --- > > Carl, > > I've gone along a different route which assures only matched messages > are touched, but it does come with quite a performance hit. > > Since there isn't a test for the actual race condition(s), I can't be > sure, but theoretically, at least one of them should be fixed now. > > Peace > > emacs/notmuch.el | 11 +++++++++-- > 1 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index f11ec24..84c3ee6 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -845,7 +845,8 @@ Each character of the tag name may consist of alphanumeric > characters as well as `_.+-'. > " > (interactive "sOperation (+add -drop): notmuch tag ") > - (let ((action-split (split-string action " +"))) > + (let ((action-split (split-string action " +")) > + (query notmuch-search-query-string)) > ;; Perform some validation > (let ((words action-split)) > (when (null words) (error "No operation given")) > @@ -853,7 +854,13 @@ characters as well as `_.+-'. > (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words)) > (error "Action must be of the form `+thistag -that_tag'")) > (setq words (cdr words)))) > - (apply 'notmuch-tag notmuch-search-query-string action-split))) > + (dolist (msgid > + (split-string > + (with-output-to-string > + (with-current-buffer standard-output > + (apply 'call-process notmuch-command nil t nil "search" "--output=messages" (list query)))) > + "\n+" t)) > + (apply 'notmuch-tag msgid action-split)))) > > (defun notmuch-search-buffer-title (query) > "Returns the title for a buffer with notmuch search results." > -- > 1.7.4.1 > Ok, even though my very first reply [1] may have created the impression that I understood the issue, I clearly didn't... The test [2] needs a more applicable commit message, and the subsequent patch [3] points more or less in the right direction, but the Message-Id list should be local to the *search buffer* rather than to the `notmuch-search-operate-all' function. `notmuch-search' could: - run "notmuch-command search" with the "--output=messages" option instead of a plain search, - maintain a buffer-local var with a list of returned Message-Id's, - ...and populate the buffer based on that list. As such we'd have -for each individual search buffer- a canonical list of Message-Id's (i.e. messages which actually *match* the query AND are currently visible in the search buffer), to be used by `notmuch-search-operate-all' et al. Peace -- Pieter [1] id:"87fwmuxxgd.fsf@praet.org" [2] id:"1309450108-2793-2-git-send-email-pieter@praet.org" [3] id:"1309450108-2793-1-git-send-email-pieter@praet.org"