8220394: bufferedStream does not honor size limit
authorstuefe
Fri, 24 May 2019 09:02:33 +0200
changeset 55023 d56d8e40b6cd
parent 55022 9785b9fb328e
child 55024 948385f851f2
8220394: bufferedStream does not honor size limit Reviewed-by: dholmes, clanger
src/hotspot/share/utilities/ostream.cpp
src/hotspot/share/utilities/ostream.hpp
test/hotspot/gtest/utilities/test_ostream.cpp
--- a/src/hotspot/share/utilities/ostream.cpp	Fri May 24 07:56:29 2019 +0100
+++ b/src/hotspot/share/utilities/ostream.cpp	Fri May 24 09:02:33 2019 +0200
@@ -939,6 +939,7 @@
   buffer_pos    = 0;
   buffer_fixed  = false;
   buffer_max    = bufmax;
+  truncated     = false;
 }
 
 bufferedStream::bufferedStream(char* fixed_buffer, size_t fixed_buffer_size, size_t bufmax) : outputStream() {
@@ -947,12 +948,17 @@
   buffer_pos    = 0;
   buffer_fixed  = true;
   buffer_max    = bufmax;
+  truncated     = false;
 }
 
 void bufferedStream::write(const char* s, size_t len) {
 
+  if (truncated) {
+    return;
+  }
+
   if(buffer_pos + len > buffer_max) {
-    flush();
+    flush(); // Note: may be a noop.
   }
 
   size_t end = buffer_pos + len;
@@ -960,19 +966,42 @@
     if (buffer_fixed) {
       // if buffer cannot resize, silently truncate
       len = buffer_length - buffer_pos - 1;
+      truncated = true;
     } else {
       // For small overruns, double the buffer.  For larger ones,
       // increase to the requested size.
       if (end < buffer_length * 2) {
         end = buffer_length * 2;
       }
-      buffer = REALLOC_C_HEAP_ARRAY(char, buffer, end, mtInternal);
-      buffer_length = end;
+      // Impose a cap beyond which the buffer cannot grow - a size which
+      // in all probability indicates a real error, e.g. faulty printing
+      // code looping, while not affecting cases of just-very-large-but-its-normal
+      // output.
+      const size_t reasonable_cap = MAX2(100 * M, buffer_max * 2);
+      if (end > reasonable_cap) {
+        // In debug VM, assert right away.
+        assert(false, "Exceeded max buffer size for this string.");
+        // Release VM: silently truncate. We do this since these kind of errors
+        // are both difficult to predict with testing (depending on logging content)
+        // and usually not serious enough to kill a production VM for it.
+        end = reasonable_cap;
+        size_t remaining = end - buffer_pos;
+        if (len >= remaining) {
+          len = remaining - 1;
+          truncated = true;
+        }
+      }
+      if (buffer_length < end) {
+        buffer = REALLOC_C_HEAP_ARRAY(char, buffer, end, mtInternal);
+        buffer_length = end;
+      }
     }
   }
-  memcpy(buffer + buffer_pos, s, len);
-  buffer_pos += len;
-  update_position(s, len);
+  if (len > 0) {
+    memcpy(buffer + buffer_pos, s, len);
+    buffer_pos += len;
+    update_position(s, len);
+  }
 }
 
 char* bufferedStream::as_string() {
--- a/src/hotspot/share/utilities/ostream.hpp	Fri May 24 07:56:29 2019 +0100
+++ b/src/hotspot/share/utilities/ostream.hpp	Fri May 24 09:02:33 2019 +0200
@@ -267,6 +267,7 @@
   size_t buffer_max;
   size_t buffer_length;
   bool   buffer_fixed;
+  bool   truncated;
  public:
   bufferedStream(size_t initial_bufsize = 256, size_t bufmax = 1024*1024*10);
   bufferedStream(char* fixed_buffer, size_t fixed_buffer_size, size_t bufmax = 1024*1024*10);
--- a/test/hotspot/gtest/utilities/test_ostream.cpp	Fri May 24 07:56:29 2019 +0100
+++ b/test/hotspot/gtest/utilities/test_ostream.cpp	Fri May 24 09:02:33 2019 +0200
@@ -67,3 +67,51 @@
 TEST_VM(ostream, stringStream_dynamic_realloc_2) {
   do_test_stringStream_dynamic_realloc(true);
 }
+
+TEST_VM(ostream, bufferedStream_static) {
+  char buf[100];
+  bufferedStream bs(buf, sizeof(buf));
+  size_t written = 0;
+  for (int i = 0; i < 100; i ++) {
+    written += print_lorem(&bs, true);
+    if (written < sizeof(buf)) {
+      ASSERT_EQ(bs.size(), written);
+    } else {
+      ASSERT_EQ(bs.size(), sizeof(buf) - 1);
+    }
+  }
+}
+
+TEST_VM(ostream, bufferedStream_dynamic_small) {
+  bufferedStream bs(1); // small to excercise realloc.
+  size_t written = 0;
+  // The max cap imposed is 100M, we should be safely below this in this test.
+  for (int i = 0; i < 10; i ++) {
+    written += print_lorem(&bs, true);
+    ASSERT_EQ(bs.size(), written);
+  }
+}
+
+/* Activate to manually test bufferedStream dynamic cap.
+
+TEST_VM(ostream, bufferedStream_dynamic_large) {
+  bufferedStream bs(1); // small to excercise realloc.
+  size_t written = 0;
+  // The max cap imposed is 100M. Writing this much should safely hit it.
+  // Note that this will assert in debug builds which is the expected behavior.
+  size_t expected_cap_at = 100 * M;
+  for (int i = 0; i < 10000000; i ++) {
+    written += print_lorem(&bs, false);
+    if (written < expected_cap_at) {
+      ASSERT_EQ(bs.size(), written);
+    } else {
+      ASSERT_EQ(bs.size(), expected_cap_at - 1);
+    }
+  }
+}
+
+*/
+
+
+
+