From 4e0dc9e37dbbafdd4f90d5764b88ded824eadb83 Mon Sep 17 00:00:00 2001
From: Chris Hobbs <chris@rx14.co.uk>
Date: Tue, 22 May 2018 16:04:27 +0100
Subject: [PATCH] Always read unbuffered when IO::Buffered#sync = true (#5849)

* Always read unbuffered when IO::Buffered#sync = true

Previously the sync option only affected writes, which wasn't optimal.

* fixup! Always read unbuffered when IO::Buffered#sync = true

* fixup! Always read unbuffered when IO::Buffered#sync = true
---
 spec/std/io/buffered_spec.cr | 100 +++++++++++++++++++++++++++++++----
 src/io/buffered.cr           |  13 +++--
 2 files changed, 101 insertions(+), 12 deletions(-)

diff --git a/spec/std/io/buffered_spec.cr b/spec/std/io/buffered_spec.cr
index 1e4d4a473fec..577cd5ebceeb 100644
--- a/spec/std/io/buffered_spec.cr
+++ b/spec/std/io/buffered_spec.cr
@@ -284,19 +284,101 @@ describe "IO::Buffered" do
     str.to_s.should eq("abcd")
   end
 
-  it "syncs" do
-    str = IO::Memory.new
+  describe "sync" do
+    it "syncs (write)" do
+      str = IO::Memory.new
 
-    io = BufferedWrapper.new(str)
-    io.sync?.should be_false
+      io = BufferedWrapper.new(str)
+      io.sync?.should be_false
+
+      io.sync = true
+      io.sync?.should be_true
+
+      io.write_byte 1_u8
+
+      str.rewind
+      str.read_byte.should eq(1_u8)
+    end
+
+    it "works with IO#read" do
+      str = IO::Memory.new "abc"
+
+      io = BufferedWrapper.new(str)
+      io.sync?.should be_false
+
+      io.sync = true
+      io.sync?.should be_true
+
+      byte = Bytes.new(1)
+      io.read_fully(byte)
+      byte[0].should eq('a'.ord.to_u8)
+
+      str.gets_to_end.should eq("bc")
+    end
+
+    it "works with IO#read (already buffered)" do
+      str = IO::Memory.new
+      str << "a" * IO::Buffered::BUFFER_SIZE
+      str.pos = 0
+
+      io = BufferedWrapper.new(str)
+      io.sync?.should be_false
+
+      IO::Buffered::BUFFER_SIZE.times do
+        byte = Bytes.new(1)
+        io.read_fully(byte)
+        byte[0].should eq('a'.ord.to_u8)
+      end
+
+      io.sync = true
+      io.sync?.should be_true
 
-    io.sync = true
-    io.sync?.should be_true
+      str << "bcde"
+      str.pos -= 4
 
-    io.write_byte 1_u8
+      byte = Bytes.new(1)
+      io.read_fully(byte)
+      byte[0].should eq('b'.ord.to_u8)
 
-    str.rewind
-    str.read_byte.should eq(1_u8)
+      str.gets_to_end.should eq("cde")
+    end
+
+    it "works with IO#read_byte" do
+      str = IO::Memory.new "abc"
+
+      io = BufferedWrapper.new(str)
+      io.sync?.should be_false
+
+      io.sync = true
+      io.sync?.should be_true
+
+      io.read_byte.should eq('a'.ord.to_u8)
+
+      str.gets_to_end.should eq("bc")
+    end
+
+    it "works with IO#read_byte (already buffered)" do
+      str = IO::Memory.new
+      str << "a" * IO::Buffered::BUFFER_SIZE
+      str.pos = 0
+
+      io = BufferedWrapper.new(str)
+      io.sync?.should be_false
+
+      IO::Buffered::BUFFER_SIZE.times do
+        io.read_byte.should eq('a'.ord.to_u8)
+      end
+
+      io.sync = true
+      io.sync?.should be_true
+
+      str << "bcde"
+      str.pos -= 4
+
+      io.read_byte.should eq('b'.ord.to_u8)
+
+      str.gets_to_end.should eq("cde")
+    end
   end
 
   it "shouldn't call unbuffered read if reading to an empty slice" do
diff --git a/src/io/buffered.cr b/src/io/buffered.cr
index 1a4320b68eb0..f2bd56df3fb9 100644
--- a/src/io/buffered.cr
+++ b/src/io/buffered.cr
@@ -33,9 +33,16 @@ module IO::Buffered
   def read_byte : UInt8?
     check_open
 
-    fill_buffer if @in_buffer_rem.empty?
+    fill_buffer if !sync? && @in_buffer_rem.empty?
     if @in_buffer_rem.empty?
-      nil
+      return nil unless sync?
+
+      byte = uninitialized UInt8
+      if read(Slice.new(pointerof(byte), 1)) == 1
+        byte
+      else
+        nil
+      end
     else
       b = @in_buffer_rem[0]
       @in_buffer_rem += 1
@@ -54,7 +61,7 @@ module IO::Buffered
       # If we are asked to read more than half the buffer's size,
       # read directly into the slice, as it's not worth the extra
       # memory copy.
-      if count >= BUFFER_SIZE / 2
+      if sync? || count >= BUFFER_SIZE / 2
         return unbuffered_read(slice[0, count]).to_i
       else
         fill_buffer