From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 10.204.141.209 with SMTP id n17cs11664bku; Sat, 3 Jul 2010 04:13:56 -0700 (PDT) Received: by 10.231.10.137 with SMTP id p9mr255288ibp.91.1278155635494; Sat, 03 Jul 2010 04:13:55 -0700 (PDT) Return-Path: Received: from rubyforge.org (rubyforge.org [205.234.109.19]) by mx.google.com with ESMTP id f14si2659498ibb.15.2010.07.03.04.13.54; Sat, 03 Jul 2010 04:13:55 -0700 (PDT) Received-SPF: pass (google.com: domain of sup-devel-bounces@rubyforge.org designates 205.234.109.19 as permitted sender) client-ip=205.234.109.19; Authentication-Results: mx.google.com; spf=pass (google.com: domain of sup-devel-bounces@rubyforge.org designates 205.234.109.19 as permitted sender) smtp.mail=sup-devel-bounces@rubyforge.org Received: from rubyforge.org (rubyforge.org [127.0.0.1]) by rubyforge.org (Postfix) with ESMTP id B32D01858367; Sat, 3 Jul 2010 07:13:54 -0400 (EDT) Received: from smtp.chost.de (setoy.chost.de [217.160.209.225]) by rubyforge.org (Postfix) with ESMTP id 73718185834E for ; Sat, 3 Jul 2010 07:07:04 -0400 (EDT) Received: (qmail 22308 invoked by uid 5015); 3 Jul 2010 11:07:07 -0000 Received: (nullmailer pid 8582 invoked by uid 8193); Sat, 03 Jul 2010 11:07:03 -0000 From: Sascha Silbe To: sup-devel Date: Sat, 3 Jul 2010 13:06:22 +0200 Message-Id: <1278155182-8545-1-git-send-email-sascha-pgp@silbe.org> X-Mailer: git-send-email 1.7.1 Mail-Followup-To: Subject: [sup-devel] [PATCH] don't leak fds for mbox sources X-BeenThere: sup-devel@rubyforge.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: Sascha Silbe , Sup developer discussion List-Id: Sup developer discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: sup-devel-bounces@rubyforge.org Errors-To: sup-devel-bounces@rubyforge.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 --- lib/sup/mbox.rb | 28 ++++++++++++++++++++++++++-- lib/sup/poll.rb | 7 ++++++- lib/sup/source.rb | 6 ++++++ 3 files changed, 38 insertions(+), 3 deletions(-) This is my first non-trivial patch to a Ruby project. Please mention even minor style mistakes. diff --git a/lib/sup/mbox.rb b/lib/sup/mbox.rb index 3f7ae3e..2bf32ea 100644 --- a/lib/sup/mbox.rb +++ b/lib/sup/mbox.rb @@ -23,7 +23,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 @@ -53,12 +53,31 @@ 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? || @path.nil? + @f.close + @f = nil + end + end + def start_offset; 0; end - def end_offset; File.size @f; end + def end_offset + @mutex.synchronize do + ensure_open + File.size @f + end + end def load_header offset header = nil @mutex.synchronize do + ensure_open @f.seek offset l = @f.gets unless MBox::is_break_line? l @@ -71,6 +90,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 @@ -88,6 +108,7 @@ class MBox < Source ## scan forward until we're at the valid start of a message def correct_offset! @mutex.synchronize do + ensure_open @f.seek cur_offset string = "" until @f.eof? || MBox::is_break_line?(l = @f.gets) @@ -100,6 +121,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 @@ -131,6 +153,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 yield @f.gets until @f.eof? || MBox::is_break_line?(l = @f.gets) @@ -145,6 +168,7 @@ class MBox < Source begin @mutex.synchronize do + ensure_open @f.seek cur_offset ## cur_offset could be at one of two places here: diff --git a/lib/sup/poll.rb b/lib/sup/poll.rb index 49e456b..3ce867c 100644 --- a/lib/sup/poll.rb +++ b/lib/sup/poll.rb @@ -159,7 +159,10 @@ EOS ## this is the primary mechanism for iterating over messages from a source. def each_message_from source, opts={} begin - return if source.done? || source.has_errors? + if source.done? || source.has_errors? + source.go_idle + return + end source.each do |offset, source_labels| if source.has_errors? @@ -175,6 +178,8 @@ EOS HookManager.run "before-add-message", :message => m yield m end + + source.go_idle rescue SourceError => e warn "problem getting messages from #{source}: #{e.message}" Redwood::report_broken_sources :force_to_top => true diff --git a/lib/sup/source.rb b/lib/sup/source.rb index 2f2e5df..13dc443 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. ## @@ -94,6 +95,11 @@ class Source ## to proactively notify the user of any source problems. def check; 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 successive offsets and labels, starting at #cur_offset. ## ## when implementing a source, you can overwrite either #each or #next. the -- 1.7.1 _______________________________________________ Sup-devel mailing list Sup-devel@rubyforge.org http://rubyforge.org/mailman/listinfo/sup-devel