Re: [PATCH 2/2] [RFC] possible solution for "Race condition for '*' command"

[Pieter Praet <pieter@praet.org>]

On Thu, 30 Jun 2011 18:08:28 +0200, Pieter Praet <pieter@praet.org> 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 <pieter@praet.org>
> ---
> 
> 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"