Skip to content
Browse files

fixes some race conditions, keeps the code terse

  • Loading branch information...
1 parent e946183 commit 6abbb13f2b6491d1993909e012ed6e05fd3845e7 Nick Kallen committed Apr 20, 2009
Showing with 25 additions and 46 deletions.
  1. +9 −21 proxy.rb
  2. +1 −3 proxy/balancers/least_connections.rb
  3. +8 −12 proxy/server.rb
  4. +7 −10 util/synchronizable.rb
View
30 proxy.rb
@@ -27,6 +27,15 @@ module ProxyServer
include LineBufferedConnection, Deferrable
extend Synchronizable
+ @@servers = (1..$options[:count]).inject([]) do |servers, i|
+ servers << Server.new($options[:host], $options[:port] + i)
+ end
+ @@balancer = $options[:balancer].new(@@servers)
+
+ def self.forward(data)
+ @@balancer.forward(data)
+ end
+
def receive_line(line)
defer do
$stats.transaction do
@@ -37,27 +46,6 @@ def receive_line(line)
end
end
end
-
- def self.forward(data)
- balancer.forward(data)
- end
-
- private
- def self.servers
- synchronize(:servers) do
- @servers ||= begin
- (1..$options[:count]).inject([]) do |servers, i|
- servers << Server.new($options[:host], $options[:port] + i)
- end
- end
- end
- end
-
- def self.balancer
- synchronize(:balancer) do
- @balancer ||= $options[:balancer].new(servers)
- end
- end
end
EM.run do
View
4 proxy/balancers/least_connections.rb
@@ -13,9 +13,7 @@ def forward(data)
def next_server
server = nil
synchronize(:next_server) do
- server = servers.min do |s1, s2|
- s1.connections <=> s2.connections
- end
+ server = servers.min
server.reserve
end
View
20 proxy/server.rb
@@ -5,18 +5,18 @@ class Server
def initialize(host, port)
@host, @port = host, port
- synchronize(:connections) { @connections = 0 }
+ @connections = 0
end
def reserve
synchronize(:connections) do
- self.connections += 1
+ @connections += 1
end
end
def release
synchronize(:connections) do
- self.connections -= 1
+ @connections -= 1
end
end
@@ -26,17 +26,13 @@ def call(data)
socket.readline
end
end
+
+ def <=>(other)
+ connections <=> other.connections
+ end
- private
- # Whilst I'm not normally a fan of closing apis or gratuitously spreading
- # private around, this code is subject to thread safety concerns, so
- # assignment should be ensure local, or the value should not be accessible,
- # instead it should have a thread safe incr! and decr!.
+ protected
def connections
@connections
end
-
- def connections=(connections)
- @connections = connections
- end
end
View
17 util/synchronizable.rb
@@ -1,19 +1,16 @@
module Synchronizable
@@mutex = Mutex.new
-
- def mutex
- # Yup, this is nasty, but it's the only way without patching
- # initialize/new and having included and extended callbacks.
+ @@mutexes = Hash.new do |h,k|
+ h[k] = Mutex.new
+ end
+
+ def mutex(mutex)
@@mutex.synchronize do
- @mutex ||= Hash.new do |h,k|
- @@mutex.synchronize do
- h[k] = Mutex.new
- end
- end
+ @@mutexes[mutex]
end
end
def synchronize(name, &block)
- mutex[name].synchronize(&block)
+ mutex(name).synchronize(&block)
end
end

1 comment on commit 6abbb13

@raggi
raggi commented on 6abbb13 Apr 21, 2009

There's still a race on the hash default.

h = Hash.new { |h,k| p h; h[k] = k }

10.times { |i|
  Thread.new {
    h[i]
  }
}

10.times { Thread.pass }

Please sign in to comment.
Something went wrong with that request. Please try again.