omfg, bug in exists? breaks world

lib/z_k/client.rb

@@ -101,7 +101,7 @@ def stat(path, opts={})
     # this method will act *exactly* like stat
     #
     def exists?(path, opts={})
-      rv = stat(path, opts={})
+      rv = stat(path, opts)
       opts[:callback] ? rv : rv.exists?
     end
 
@@ -184,18 +184,26 @@ def rm_rf(paths)
     # will block the caller until +abs_node_path+ has been removed
     def block_until_node_deleted(abs_node_path)
       queue = Queue.new
+      ev_sub = nil
 
-      node_deletion_cb = lambda do
-        unless exists?(abs_node_path, :watch => true)
+      node_deletion_cb = lambda do |event|
+        if event.node_deleted?
           queue << :locked
+        else
+          queue << :locked unless exists?(abs_node_path, :watch => true)
         end
       end
 
-      watcher.register(abs_node_path, &node_deletion_cb)
-      node_deletion_cb.call
+      ev_sub = watcher.register(abs_node_path, &node_deletion_cb)
+
+      # set up the callback, but bail if we don't need to wait
+      return true unless exists?(abs_node_path, :watch => true)  
 
       queue.pop # block waiting for node deletion
       true
+    ensure
+      # be sure we clean up after ourselves
+      ev_sub.unregister if ev_sub
     end
 
     # creates a new locker based on the name you send in

spec/client_spec.rb

@@ -47,6 +47,55 @@
       end
     end
   end
+
+  describe :block_until_node_deleted do
+    before do
+      @path = '/_bogualkjdhsna'
+    end
+
+    describe 'no node initially' do
+      before do
+        @zk.exists?(@path).should be_false
+      end
+
+      it %[should not block] do
+        @a = false
+
+        th = Thread.new do
+          @zk.block_until_node_deleted(@path)
+          @a = true
+        end
+
+        th.join(2)
+        @a.should be_true
+      end
+    end
+
+    describe 'node exists initially' do
+      before do
+        @zk.create(@path, '', :mode => :ephemeral)
+        @zk.exists?(@path).should be_true
+      end
+
+      it %[should block until the node is deleted] do
+        @a = false
+
+        th = Thread.new do
+          @zk.block_until_node_deleted(@path)
+          @a = true
+        end
+
+        Thread.pass
+        @a.should be_false
+
+        @zk.delete(@path)
+
+        wait_until(2) { @a }
+        @a.should be_true
+      end
+    end
+  end
 end
 
 
+

spec/shared_locker_spec.rb

@@ -111,6 +111,7 @@
 
           th.join(2)
 
+          wait_until(2) { !ary.empty? }
           ary.length.should == 1
 
           @read_locker.should be_locked

spec/zookeeper_spec.rb

@@ -30,7 +30,7 @@ def delete_test!
     end
 
     it "should not exist" do
-      @zk.exists?("/test").should be_nil
+      @zk.exists?("/test").should be_false
     end
 
     it "should create a path" do
@@ -62,7 +62,7 @@ def delete_test!
 
       @zk = ZK.new("localhost:#{ZK_TEST_PORT}", :watcher => nil)
       wait_until{ @zk.connected? }
-      @zk.exists?("/test").should be_nil
+      @zk.exists?("/test").should be_false
     end
 
     it "should remove sequential ephemeral path when client session ends" do
@@ -73,7 +73,7 @@ def delete_test!
 
       @zk = ZK.new("localhost:#{ZK_TEST_PORT}", :watcher => nil)
       wait_until{ @zk.connected? }
-      @zk.exists?(created).should be_nil
+      @zk.exists?(created).should be_false
     end
 
   end
@@ -100,7 +100,11 @@ def delete_test!
     end
 
     it "should return a stat" do
-      @zk.exists?("/test").should be_instance_of(ZookeeperStat::Stat)
+      @zk.stat("/test").should be_instance_of(ZookeeperStat::Stat)
+    end
+
+    it "should return a boolean" do
+      @zk.exists?("/test").should be_true
     end
 
     it "should get data and stat" do
@@ -118,7 +122,7 @@ def delete_test!
 
     it "should delete path" do
       @zk.delete("/test")
-      @zk.exists?("/test").should be_nil
+      @zk.exists?("/test").should be_false
     end
 
     it "should create a child path" do