From 2b0de279565c80d9bf30301e6e4ecf9a9f5f0880 Mon Sep 17 00:00:00 2001
From: Holger Just <hello@holgerjust.de>
Date: Thu, 27 Apr 2017 22:04:11 +0200
Subject: [PATCH] Return nil on BufferStacl#flush_and_pop

The buffer should not be reused after a flush anyway. By not returning
it, we ensure that it does not leak outside.
---
 lib/rackstash/buffer_stack.rb       |  9 ++++-----
 spec/rackstash/buffer_stack_spec.rb | 19 ++++++++++++-------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/lib/rackstash/buffer_stack.rb b/lib/rackstash/buffer_stack.rb
index 5037389..71cf15e 100644
--- a/lib/rackstash/buffer_stack.rb
+++ b/lib/rackstash/buffer_stack.rb
@@ -51,12 +51,11 @@ module Rackstash
     # If there was a buffer on the stack and it has pending data, it is flushed
     # to the {#sink} before it is returned.
     #
-    # @return [Buffer, nil] the popped and flushed {Buffer} or `nil` if there
-    #   was no Buffer in the stack
+    # @return [nil]
     def flush_and_pop
-      @stack.pop.tap do |buffer|
-        buffer.flush if buffer
-      end
+      buffer = @stack.pop
+      buffer.flush if buffer
+      nil
     end
   end
 end
diff --git a/spec/rackstash/buffer_stack_spec.rb b/spec/rackstash/buffer_stack_spec.rb
index afe09e9..df9fc20 100644
--- a/spec/rackstash/buffer_stack_spec.rb
+++ b/spec/rackstash/buffer_stack_spec.rb
@@ -23,13 +23,12 @@ describe Rackstash::BufferStack do
 
     it 'adds a new implicit buffer' do
       expect(stack.current).to be_a Rackstash::Buffer
-      expect(stack.flush_and_pop).to be_a Rackstash::Buffer
+      stack.flush_and_pop
 
-      # no further Buffer on the stack
-      expect(stack.flush_and_pop).to be nil
+      expect(stack.instance_variable_get(:@stack).count).to eql 0
 
       expect(stack.current).to be_a Rackstash::Buffer
-      expect(stack.flush_and_pop).to be_a Rackstash::Buffer
+      expect(stack.instance_variable_get(:@stack).count).to eql 1
     end
   end
 
@@ -63,9 +62,15 @@ describe Rackstash::BufferStack do
         .to change { stack.instance_variable_get(:@stack).count }.from(1).to(0)
     end
 
-    it 'returns the removed buffer' do
+    it 'does nothing if there is no buffer' do
+      expect(stack.instance_variable_get(:@stack).count).to eql 0
+      expect { stack.flush_and_pop }
+        .not_to change { stack.instance_variable_get(:@stack) }
+    end
+
+    it 'always returns nil' do
       new_buffer = stack.push
-      expect(stack.flush_and_pop).to equal new_buffer
+      expect(stack.flush_and_pop).to be nil
       expect(stack.flush_and_pop).to be nil
     end
 
@@ -75,7 +80,7 @@ describe Rackstash::BufferStack do
       expect(new_buffer).to receive(:flush).once
 
       stack.flush_and_pop
-      stack.flush_and_pop # nil, thus `#flush` is not called again
+      stack.flush_and_pop # no further buffer, thus `#flush` is not called again
     end
   end
 end