[PATCH] CGI::Session.rb

I'm sorry, I found a typo in my previous version of the patch.  Here
follows the corrected one. 

BTW, if I'm stepping on any toes, I don't mean to offend; I'm just
trying to avoid having to apply this patch every time a new version of
Ruby is released.  If there's a better way to propose this patch,
please let me know.

My goal with this patch is to only actually keep the file open when
it's being used (to update, or restore).  The current version keeps it
open all the time, which can cause problems when you fork off a
long-running CGI process, because the lock is acquired when
CGI::Session#restore is called, but never released.  Thus, the next
time a user requests a CGI script under the session, the request hangs
as the second process tries to call File#flock.

Two things help here: one, read locks should always use LOCK_SH, and
only write locks need LOCK_EX.  The other, of course, is that the file
itself is never held open longer than strictly necessary.  There's
still a concurrency issue with two processes potentially holding
different ideas of what's in the session at one time, but that's a
problem with the current session.rb, so at least I'm not making it
worse.

-=Eric

--- session-old.rb	2004-01-26 17:24:25.000000000 -0700
+++ session.rb	2004-01-27 12:29:41.000000000 -0700
@@ -364,16 +364,11 @@
 	unless check_id(id)
 	  raise ArgumentError, "session_id `%s' is invalid" % id
 	end
-	path = dir+"/"+prefix+id
-	path.untaint
-	unless File::exist? path
+	@path = dir+"/"+prefix+id
+	@path.untaint
+	unless File::exist? @path
 	  @hash = {}
 	end
-	begin
-	  @f = open(path, "r+")
-	rescue Errno::ENOENT
-	  @f = open(path, "w+")
-	end
       end
 
       # Restore session state from the session's FileStore file.
@@ -382,13 +377,17 @@
       def restore
 	unless @hash
 	  @hash = {}
-	  @f.flock File::LOCK_EX
-	  @f.rewind
-	  for line in @f
-	    line.chomp!
-	    k, v = line.split('=',2)
-	    @hash[CGI::unescape(k)] = CGI::unescape(v)
-	  end
+          begin
+	    f = File.open(@path, 'r')
+	    f.flock File::LOCK_SH
+	    for line in f
+	      line.chomp!
+	      k, v = line.split('=',2)
+	      @hash[CGI::unescape(k)] = CGI::unescape(v)
+	    end
+          ensure
+	    f.close unless f.nil?
+          end
 	end
 	@hash
       end
@@ -396,25 +395,25 @@
       # Save session state to the session's FileStore file.
       def update
 	return unless @hash
-	@f.rewind
-	for k,v in @hash
-	  @f.printf "%s=%s\n", CGI::escape(k), CGI::escape(String(v))
-	end
-	@f.truncate @f.tell
+        begin
+	  f = File.open(@path, 'w')
+	  f.flock File::LOCK_EX
+   	  for k,v in @hash
+	    f.printf "%s=%s\n", CGI::escape(k), CGI::escape(String(v))
+	  end
+        ensure
+          f.close unless f.nil?
+        end
       end
 
       # Update and close the session's FileStore file.
       def close
-	return if @f.closed?
 	update
-	@f.close
       end
 
       # Close and delete the session's FileStore file.
       def delete
-	path = @f.path
-	@f.close
-	File::unlink path
+	File::unlink @path
       end
     end