[darcs-users] darcs patch: Handle empty files in mmapFilePS. Also cater for FD sh...

Judah Jacobson judah.jacobson at gmail.com
Wed Feb 4 22:53:47 UTC 2009


On Wed, Feb 4, 2009 at 11:02 AM, Petr Rockai <me at mornfall.net> wrote:
> Eric Kow <kowey at darcs.net> writes:
>> But, first, could you please check on the possible risks on our other
>> use of the catch function in the ByteStringUtils module?  It looks like
>> there is only one.
> Well, I did have a look before sending and I believe it's safe. I still don't
> really see through the muddy waters of haskell exceptions that clearly
> though. It would make sense to hear a comment from an expert. From
> Control.Exception haddocks, I gather that it's always safe to replace
> Prelude.catch with Control.Exception.catch (under the condition you didn't want
> some exceptions to fly through the catch unnoticed, but that would be very
> dubious use of catch anyway).
>
>> Handle empty files in mmapFilePS. Also cater for FD shortage.
>> -------------------------------------------------------------
>>> +  x <- unsafeMMapFile f
>>> +   `catch` (\_ -> do
>>> +                     size <- fileSize `fmap` getSymbolicLinkStatus f
>>> +                     if size == 0
>>> +                        then return B.empty
>>> +                        else performGC >> unsafeMMapFile f)
>>
>> Second: will there be any weird performance consequences if we're
>> calling performGC on every mmapFilePS?
> It's in the catch handler. This code trips only if unsafeMMapFile fails for
> reason other than empty file (which *usually* is the open fd limit). The size
> check is fairly expensive to do as well, that's why I only do it when
> unsafeMMapFile fails (which is rather cheaper than the file size check, due to
> (IMHO) stupid System.Posix.File implementation involving unsafePerformIO).
>

I think the below code might be slightly cleaner:

x <- unsafeMMapFile `catch` $ \e -> do
        size <- fileSize `fmap` getSymbolicLinkStatus f
        if size == 0
            then return B.empty
            else throwIO e

That is, explicitly re-throw the caught exception, rather than calling
unsafeMMapFile again to get it implicitly re-thrown.

Though, for this specific situation it probably doesn't make a big
difference which way you do it.

-Judah


More information about the darcs-users mailing list