[PATCH 2 of 4] add filteropener abstraction for store openers

Dan Villiom Podlaski Christiansen danchr at gmail.com
Mon May 2 03:39:34 CDT 2011


On 2 May 2011, at 09:46, Adrian Buehlmann wrote:

> On 2011-05-02 09:10, Dan Villiom Podlaski Christiansen wrote:
>> On 2 May 2011, at 00:47, Adrian Buehlmann wrote:
>>
>>> So, the openers of basicstore and encodedstore are now derived from
>>> that
>>> new abstractopener (via filteropener), but the opener of
>>> fncachestore is
>>> still the good old function object as defined on line 378 in  
>>> store.py.
>>
>> Thanks for catching that one; I'll send a followup to fix it shortly.
>
> I have a (possibly silly, I don't know) followup question (without a
> hidden agenda behind it -- really):
>
> Why does everything work with PyPy (IIRC, as you said?), if these
> openers don't get those extra functions from that new base class?

To be fair, it's not quite *everything* that works with PyPy. But most  
things do, though ;-)

> Which leads to asking myself: do the store openers really have to  
> derive
> from the abstract base? After all, it seems fncachestore works without
> deriving this opener from the abstract base?
>
> It seems to me, the store openers are generally treated quite  
> politely,
> meaning: revlog and friends are already carefully closing files,  
> aren't
> they?

The stores may close some of the files opened, but probably not *all*  
of them. On some occasions, everything works just fine despite closing  
(and flushing) the file handle being delayed for a short while. In  
other cases it can either lead to corruption of the files or to file  
descriptor exhaustion.

That is why I wanted *all* openers to have these methods. It's fairly  
hard to predict whether a transient leak of a file handle is safe, so  
it'd be better if we had a simple pattern for dealing with this.

The reason everything (appeared to) work with the fncacheopener being  
a simple function is that none of the calls to .read(), .write()  
or .append() I added affected it. Someone might change that in the  
future, so it was arguably a bug.

I think it's unlikely that I've found *all* cases of files that aren't  
closed immediately; many probably remain, and some might even be  
problematic.

--

Dan Villiom Podlaski Christiansen
danchr at gmail.com



More information about the Mercurial-devel mailing list