[PATCH] tags: don't allow environment errors to be raised from _writetagscache

[Adrian Buehlmann]

On 2011-06-17 08:54, Adrian Buehlmann wrote:
> On 2011-06-17 01:12, Steve Borho wrote:
>> On Thu, Jun 16, 2011 at 3:45 PM, Adrian Buehlmann <adrian at cadifra.com> wrote:
>>> On 2011-06-16 22:26, Steve Borho wrote:
>>>> # HG changeset patch
>>>> # User Steve Borho <steve at borho.org>
>>>> # Date 1308255977 18000
>>>> # Node ID 4196f029b6b4a1f05dd0eac9a9df9d19424af4cc
>>>> # Parent  ba3c36cea66e7a9cc0bb274b7f7b73ad65212df7
>>>> tags: don't allow environment errors to be raised from _writetagscache
>>>>
>>>> See https://bitbucket.org/tortoisehg/thg/issue/719
>>>
>>> What is an "environment error"?
>>
>> EnvironmentError is the base class of IOError and OSError.
>>
>> http://docs.python.org/library/exceptions.html
> 
> ok
> 
>>>> diff -r ba3c36cea66e -r 4196f029b6b4 mercurial/tags.py
>>>> --- a/mercurial/tags.py       Thu Jun 16 14:33:06 2011 -0500
>>>> +++ b/mercurial/tags.py       Thu Jun 16 15:26:17 2011 -0500
>>>> @@ -286,4 +286,7 @@
>>>>      for (name, (node, hist)) in cachetags.iteritems():
>>>>          cachefile.write("%s %s\n" % (hex(node), name))
>>>>
>>>> -    cachefile.rename()
>>>> +    try:
>>>> +        cachefile.rename()
>>>> +    except (OSError, IOError):
>>>> +        pass
>>>
>>> This still feels like papering over a problem not clearly understood (at
>>> least on my part).
>>
>> It could be, but this still leads to exceptions being raised from just
>> about any API call.  The tags cache is updated from a lot of common
> 
> ..
> 
>> code paths.  We already capture and ignore exceptions when we create
>> the tempfile (failure to write the cache file is not a fatal error).
> 
> Yes, this capture is fine and it should cover the later rename already
> for the cases we cover with that (not being able to write to the target
> directory in general).
> 
>> This is just acknowledging that on Windows it is often the rename that
>> throws the error.
> 
> I haven't seen a conclusive theory yet. And I would like to see one
> before papering over that error and risking later getting complaints
> that Mercurial is slow. Later discovering that the cache is defunct
> *because* we papered over the error is much harder.
> 
>>> And, in total, I've pondered about file access on Windows for quite a
>>> while now.
>>
>> It is most likely a bad virus checker, but the people who file these
>> bugs are quite un-responsive to questions.  I've lost count of how
>> many of these we've received (bitbucket search finds 8 issues, but
>> some bug reports are attached and aren't searchable).

I forgot to add that I *am* reading the TortoiseHg bugreports coming in
and the tracebacks pasted in particular.

It's not like we get reports of this class of error on a daily basis and
it doesn't really feel like we get tens of them per month - Even though
I don't have statistics here myself.

Compare this with the >14000 downloads for the thg x64 installer alone
(in two weeks).

> The problem I have is the term "most likely". I think it's a sign that
> we haven't understood the real cause of the problem yet.
> 
> Today, there are two camps of virus checkers on Windows
> 
> (A) opening files with shared delete flag (allows renames and unlinks)
> (B) opening files without or locking them by mapping into RAM
> 
> My goal is to make Mercurial work fine in the presence of (A) and I have
> even installed one of these here myself for that purpose and I've never
> seen that traceback here.
> 
> Mercurial is already pretty much compatible with class (A) checkers and
> claiming such a problem is a problem with a virus checker doesn't work
> that well any more these days (with current Mercurial).
> 
> At least I would like to see what we get with 1.9 on that front, that
> is, releasing 1.9 without papering over this problem. Or I would like to
> see a theory why the error happens before papering over it.