Merge pull request #905 from andrykonchin/fix-priority-queue

Fix RubyNonConcurrentPriorityQueue#delete method

Author: pitr-ch

lib/concurrent-ruby/concurrent/collection/ruby_non_concurrent_priority_queue.rb

@@ -30,7 +30,7 @@ def delete(item)
           if @queue[k] == item
             swap(k, @length)
             @length -= 1
-            sink(k)
+            sink(k) || swim(k)
             @queue.pop
           else
             k += 1
@@ -126,12 +126,17 @@ def ordered?(x, y)
       # 
       # @!visibility private
       def sink(k)
+        success = false
+
         while (j = (2 * k)) <= @length do
           j += 1 if j < @length && ! ordered?(j, j+1)
           break if ordered?(k, j)
           swap(k, j)
+          success = true
           k = j
         end
+
+        success
       end
 
       # Percolate up to maintain heap invariant.
@@ -140,10 +145,15 @@ def sink(k)
       # 
       # @!visibility private
       def swim(k)
+        success = false
+
         while k > 1 && ! ordered?(k/2, k) do
           swap(k, k/2)
           k = k/2
+          success = true
         end
+
+        success
       end
     end
   end

spec/concurrent/collection/non_concurrent_priority_queue_spec.rb

@@ -125,6 +125,41 @@
     it 'returns false when called on an empty queue' do
       expect(subject.delete(:foo)).to be_falsey
     end
+
+    def dequeue_all(queue)
+      queue.size.times.inject([]) do |acc, _|
+        acc << queue.pop
+      end
+    end
+
+    it 'deletes the requested item when it is "smaller" than the last element' do
+      [
+        100,
+        9, 90,
+        7, 8, 70, 80,
+        3, 4, 5, 6, 30, 40, 50, 60
+      ].each do |item|
+        subject << item
+      end
+
+      subject.delete(8)
+
+      expect(subject.length).to eq 14
+      expect(subject.pop).to eq 100
+      expect(subject.pop).to eq 90
+      expect(subject.pop).to eq 80
+      expect(subject.pop).to eq 70
+      expect(subject.pop).to eq 60
+      expect(subject.pop).to eq 50
+      expect(subject.pop).to eq 40
+      expect(subject.pop).to eq 30
+      expect(subject.pop).to eq 9
+      expect(subject.pop).to eq 7
+      expect(subject.pop).to eq 6
+      expect(subject.pop).to eq 5
+      expect(subject.pop).to eq 4
+      expect(subject.pop).to eq 3
+    end
   end
 
   context '#empty?' do