Generate unique, temporary names for socket files in a more robust way.

CHANGELOG.md

@@ -1,3 +1,8 @@
+Next Release
+============
+
+* Improve generation of temporary filename for socket - [@joeyAghion](https://github.com/joeyAghion).
+
 0.3.0
 =====
 

lib/heroku/forward.rb

@@ -2,3 +2,4 @@
 require 'heroku/forward/errors'
 require 'heroku/forward/backends'
 require 'heroku/forward/proxy'
+require 'heroku/forward/utils/dir'

lib/heroku/forward/backends/thin.rb

@@ -22,7 +22,7 @@ class Thin
         #  ssl_verify:    activated with  ssl_verify
         def initialize(options = {})
           @application = options[:application]
-          @socket = options[:socket] || new_socket
+          @socket = options[:socket] || Heroku::Forward::Utils::Dir.tmp_filename('thin', '.sock')
           @env = options[:env] || :development
 
           @ssl = options[:ssl] || false
@@ -62,16 +62,10 @@ def spawned?
 
         private
 
-          def new_socket
-            Tempfile.open 'thin' do |file|
-              return file.path
-            end
-          end
-
-          def check!
-            raise Heroku::Forward::Errors::MissingBackendOptionError.new('application') unless @application && @application.length > 0
-            raise Heroku::Forward::Errors::MissingBackendApplicationError.new(@application) unless File.exists?(@application)
-          end
+        def check!
+          raise Heroku::Forward::Errors::MissingBackendOptionError.new('application') unless @application && @application.length > 0
+          raise Heroku::Forward::Errors::MissingBackendApplicationError.new(@application) unless File.exists?(@application)
+        end
 
       end
     end

lib/heroku/forward/backends/unicorn.rb

@@ -6,7 +6,7 @@ class Unicorn
 
         def initialize(options = {})
           @application = options[:application]
-          @socket = options[:socket] || tmp_filename('unicorn', '.sock')
+          @socket = options[:socket] || Heroku::Forward::Utils::Dir.tmp_filename('unicorn', '.sock')
           @env = options[:env] || 'development'
           @config_file = options[:config_file]
         end
@@ -38,11 +38,6 @@ def spawned?
 
         private
 
-        # Borrowed from ruby 1.9's Dir::Tmpname.make_tmpname for 1.8.7-compatibility.
-        def tmp_filename(prefix, suffix)
-          File.join Dir.tmpdir, "#{prefix}#{Time.now.strftime("%Y%m%d")}-#{$$}-#{rand(0x100000000).to_s(36)}#{suffix}"
-        end
-
         def check!
           raise Heroku::Forward::Errors::MissingBackendOptionError.new('application') unless @application && @application.length > 0
           raise Heroku::Forward::Errors::MissingBackendApplicationError.new(@application) unless File.exists?(@application)

lib/heroku/forward/utils/dir.rb

@@ -0,0 +1,16 @@
+module Heroku
+  module Forward
+    module Utils
+
+      module Dir
+
+        # Partially borrowed from ruby 1.9's Dir::Tmpname.make_tmpname for 1.8.7-compatibility.
+        def self.tmp_filename(prefix, suffix)
+          File.join ::Dir.tmpdir, "#{prefix}#{Time.now.strftime("%Y%m%d")}-#{$$}-#{rand(0x100000000).to_s(36)}#{suffix}"
+        end
+
+      end
+
+    end
+  end
+end

spec/lib/heroku/backends/thin_spec.rb → spec/lib/heroku/forward/backends/thin_spec.rb

@@ -7,10 +7,18 @@
     let(:backend) do
       Heroku::Forward::Backends::Thin.new
     end
+
+    after do
+      backend.terminate!
+    end
 
     it "#spawned?" do
       backend.spawned?.should be_false
     end
+
+    it "doesn't generate socket file" do
+      File.exists?(backend.socket).should_not be_true
+    end
 
     context "checks" do
       it "checks for application" do
@@ -27,13 +35,26 @@
       end
 
     end
-
-    it "#spawn!" do
-      backend.application = "spec/support/app.ru"
-      backend.spawn!.should_not == 0
-      sleep 2
-      backend.terminate!.should be_true
+
+    describe "#spawn!" do
+      before do
+        backend.application = "spec/support/app.ru"
+      end
+
+      it "starts successfully" do
+        backend.spawn!.should_not == 0
+        sleep 2
+        backend.terminate!.should be_true
+      end
+
+      it "generates socket file that outlives garbage-collection" do
+        backend.spawn!
+        sleep 2
+        lambda { GC.start }.should_not raise_error
+        File.exists?(backend.socket).should be_true
+      end
     end
+
   end
 
   describe "with SSL" do

spec/lib/heroku/forward/backends/unicorn_spec.rb

@@ -0,0 +1,61 @@
+require 'spec_helper'
+require 'heroku/forward/backends/unicorn'
+
+describe Heroku::Forward::Backends::Unicorn do
+
+  describe "with defaults" do
+    let(:backend) do
+      Heroku::Forward::Backends::Unicorn.new
+    end
+
+    after do
+      backend.terminate!
+    end
+
+    it "#spawned?" do
+      backend.spawned?.should be_false
+    end
+
+    context "checks" do
+      it "checks for application" do
+        expect {
+          backend.spawn!
+        }.to raise_error Heroku::Forward::Errors::MissingBackendOptionError
+      end
+
+      it "checks that the application file exists" do
+        expect {
+          backend.application = 'spec/foobar'
+          backend.spawn!
+        }.to raise_error Heroku::Forward::Errors::MissingBackendApplicationError
+      end
+
+    end
+
+    it "#spawn!" do
+      backend.application = "spec/support/app.ru"
+      backend.spawn!.should_not == 0
+      sleep 2
+      backend.terminate!.should be_true
+    end
+  end
+
+  context "constructs command" do
+
+    let(:backend) do
+      Heroku::Forward::Backends::Unicorn.new(
+        :application => 'spec/support/app.ru',
+        :env => 'test',
+        :socket => '/tmp/unicorn.sock',
+        :config_file => 'spec/support/unicorn.rb'
+      )
+    end
+
+    it "forwards arguments to spawner" do
+      Spoon.should_receive(:spawnp).with(*%w{unicorn --env test --config-file spec/support/unicorn.rb --listen /tmp/unicorn.sock spec/support/app.ru}).and_return(0)
+      backend.spawn!
+    end
+
+  end
+end
+

spec/lib/heroku/forward/utils/dir_spec.rb

@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe Heroku::Forward::Utils::Dir do
+
+  context "#tmp_filename" do
+    let(:filename) { Heroku::Forward::Utils::Dir.tmp_filename('foo', '.bar') }
+
+    it "doesn't create file" do
+      File.exists?(filename).should_not be_true
+    end
+
+    it "names file using prefix and suffix" do
+      filename.should match(/foo.*\.bar/)
+    end
+  end
+
+end
+