[sup-devel] [PATCH v2] don't leak fds for mbox sources

[Sascha Silbe <sascha-pgp@silbe.org>]

Only open the mbox when it's actually used and close it again once we're
finished iterating over it.

Adds a method go_idle to Source to inform the source we're unlikely to
use it in the near future so it can do internal clean-up as appropriate.

Signed-off-by: Sascha Silbe <sascha-pgp@silbe.org>
---
v1->v2: rebased on next

 lib/sup/mbox.rb   |   22 ++++++++++++++++++++--
 lib/sup/poll.rb   |    2 ++
 lib/sup/source.rb |    6 ++++++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/lib/sup/mbox.rb b/lib/sup/mbox.rb
index b03a99c..78f7296 100644
--- a/lib/sup/mbox.rb
+++ b/lib/sup/mbox.rb
@@ -22,7 +22,7 @@ class MBox < Source
       raise ArgumentError, "not an mbox uri" unless uri.scheme == "mbox"
       raise ArgumentError, "mbox URI ('#{uri}') cannot have a host: #{uri.host}" if uri.host
       raise ArgumentError, "mbox URI must have a path component" unless uri.path
-      @f = File.open uri.path, 'rb'
+      @f = nil
       @path = uri.path
     else
       @f = uri_or_fp
@@ -45,9 +45,23 @@ class MBox < Source
     end
   end
 
+  def ensure_open
+    @f = File.open @path, 'rb' if @f.nil?
+  end
+  private :ensure_open
+
+  def go_idle
+    @mutex.synchronize do
+      return if @f.nil? or @path.nil?
+      @f.close
+      @f = nil
+    end
+  end
+
   def load_header offset
     header = nil
     @mutex.synchronize do
+      ensure_open
       @f.seek offset
       header = parse_raw_email_header @f
     end
@@ -56,6 +70,7 @@ class MBox < Source
 
   def load_message offset
     @mutex.synchronize do
+      ensure_open
       @f.seek offset
       begin
         ## don't use RMail::Mailbox::MBoxReader because it doesn't properly ignore
@@ -74,6 +89,7 @@ class MBox < Source
   def raw_header offset
     ret = ""
     @mutex.synchronize do
+      ensure_open
       @f.seek offset
       until @f.eof? || (l = @f.gets) =~ /^\r*$/
         ret << l
@@ -105,6 +121,7 @@ class MBox < Source
   ## sup-sync-back has to do it.
   def each_raw_message_line offset
     @mutex.synchronize do
+      ensure_open
       @f.seek offset
       until @f.eof? || MBox::is_break_line?(l = @f.gets)
         yield l
@@ -118,7 +135,7 @@ class MBox < Source
 
   def poll
     first_offset = first_new_message
-		offset = first_offset
+    offset = first_offset
     end_offset = File.size @f
     while offset and offset < end_offset
       yield :add,
@@ -131,6 +148,7 @@ class MBox < Source
 
   def next_offset offset
     @mutex.synchronize do
+      ensure_open
       @f.seek offset
       nil while line = @f.gets and not MBox::is_break_line? line
       offset = @f.tell
diff --git a/lib/sup/poll.rb b/lib/sup/poll.rb
index a17a199..afd3d95 100644
--- a/lib/sup/poll.rb
+++ b/lib/sup/poll.rb
@@ -182,6 +182,8 @@ EOS
           end
         end
       end
+
+      source.go_idle
     rescue SourceError => e
       warn "problem getting messages from #{source}: #{e.message}"
     end
diff --git a/lib/sup/source.rb b/lib/sup/source.rb
index ebda6b8..204ebd5 100644
--- a/lib/sup/source.rb
+++ b/lib/sup/source.rb
@@ -40,6 +40,7 @@ class Source
   ## - raw_header offset
   ## - raw_message offset
   ## - check (optional)
+  ## - go_idle (optional)
   ## - next (or each, if you prefer): should return a message and an
   ##   array of labels.
   ##
@@ -81,6 +82,11 @@ class Source
 
   def read?; false; end
 
+  ## release resources that are easy to reacquire. it is called
+  ## after processing a source (e.g. polling) to prevent resource
+  ## leaks (esp. file descriptors).
+  def go_idle; end
+
   ## Yields values of the form [Symbol, Hash]
   ## add: info, labels, progress
   ## delete: info, progress
-- 
1.7.2.3

_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel