my first guess is that we are keeping a bunch of streams open and they aren't being properly disposed.  The file here appears to be a crw file, are a lot of the files you are importing raw files?  Is so what camera created them?

I have a bunch of jpg files, but the problem seems to be with the crw, these are raw files created by a Canon 300D camera. I also have cr2 files created by a Canon 350D, but at the time f-spot crashed had not started to process those cr2 files.

I've committed a change that should reduce the severity of the problem to CVS can you try it out and let me know if it helps things?

*** Bug 402336 has been marked as a duplicate of this bug. ***

that "too many open files" still happens with 0.3.x, as reported in bug #402336

FYI This is on 0.3.2 on Ubuntu.

Hi Larry, Stephane:

In response to how much is "too much" in #402336, I'm trying to re-import 50,000 photographs.  Trying to import from my backup drive didn't complete, so I tried breaking it down by year -- i.e., import photos from 2000, 2001, ... up to today.

My JPEGs (2000-2003) were successful however importing RAWs (2004-today) never succeeds.  I have to import by month (i.e., 2005/01/02) and at times even that doesn't work if the directory contains a few hundred RAWs.

Yeah it right now the in the ciff loader the stream is being left open indefinitely.

Is there a fix available in svn for this yet?  F-Spot is really unusable after importing 230 or so photos.

Created attachment 84402 [details] [review]
Added new Dispose method to ImageFile

This is a patch that adds a Dispose method to the ImageFile class.  This bug has annoyed me for months and has made me stop taking photographs because F-Spot is crucial in my workflow.  I have some time off before starting a new job so here we go...

I've grepped through the main src tree looking for references to ImageFile instances and dispose of them when necessary.  The ImageFile.Dispose method is virtual and does nothing by default.  The CiffFile class is the only overrider which will Close the Stream reference it keeps.

As for testing, pre-patch, I can do an lsof on f-spot and see hundreds of references, sometimes duplicate, to RAW files.  Post-patch, I see at most one reference.  It appears to work however YMMV.

Enjoy.

Hi George,

good catch !

Here's some comments, in order to write better code:

1) If you add a Dispose method to a class, make the class IDisposable
    class ImageFile: IDisposable

2) if there's no default destructors (almost never the case in C#), you can tell the JIT to GC your object in the Dispose() call (saves some precious cycles):
    public void Dispose ()
    {
        System.GC.SuppressFinalize (this);
    }

3) use the 'using' statement wherever you can (15.13 in monodoc). In short, the 'using' statement automatically call Dispose for you. So this:

    Img img = new Img ();
    img.Foo();
    img.Dispose();

is (almost) equivalent to this:

    using (Img img = new Img() {
        img.Foo ()
    }

Always prefer the 'using' syntax if possible. I said 'almost equivalent', cause the 'using' statement Dispose() the object even if an exception is thrown in the nested code. So, you don't have to add the 'finally' exception handlers...

Hope it helps...

Created attachment 84435 [details] [review]
Re-jiggle code to use IDispose

How's this....

Created attachment 84476 [details] [review]
Fix to AsyncPixbufLoader to use using.

This patch also fixes the bug reported in #410168.  Enjoy.

Hi George,

you rock !!

Do you want an extra point for style ? but you don't have to rewrite your patch just for this...

this code:

using (ImageFile img = ImageFile.Create (xxxxx)) {
        using (Gdk.Pixbuf pixbuf = img.Load ()) {
                pixbuf.Foo ();
        }
}

is equivalent to this one:

using (ImageFile img = ImageFile.Create (xxxxx), Gdk.Pixbuf pixbuf = img.Load ()) {
        pixbuf.Foo ();
}

I'm seriously thinking about committing it quickly

regards

s

Stephane, feel free to make the necessary changes and re-patch.  I have a working F-Spot on my system now and I have 50,000 photographs I need to bulk import!

a slightly different patch is committed in r3029. thx again George.

George, Thank You, I tested the build 3031 from svn, and works great!, I'm back using f-spot again.

Happy to help.  FWIW I've been using the same instance of F-Spot for 3 days without a crash.