RFR(XL): 8181917: Refactor UL LogStreams to avoid using resource area

[Erik Helin]

Hi Thomas,

On 06/30/2017 08:32 PM, Thomas Stüfe wrote:
> Hi Eric,
>
> thank you for the review!
>
> New
> Version: http://cr.openjdk.java.net/~stuefe/webrevs/8181917-refactor-ul-logstream/all.webrev.03/webrev/

I think you still have a problem with "runaway" loggers in this version:

+// try_ensure_cap tries to enlarge the capacity of the internal buffer
+// to the given atleast value. May fail if either OOM happens or atleast
+// is larger than a reasonable max of 1 M. Caller must not assume
+// capacity without checking.
+void LogStream::LineBuffer::try_ensure_cap(size_t atleast) {
+  assert(_cap >= sizeof(_smallbuf), "sanity");
+  if (_cap < atleast) {
+    const size_t reasonable_max = 1 * M;
+    size_t newcap = align_size_up(atleast + 64, 64);
+    assert(_cap <= reasonable_max, "sanity");
+    // Cap out at a reasonable max to prevent runaway leaks.
+    if (newcap > reasonable_max) {
+      newcap = reasonable_max;
+    }
+
+    char* const newbuf = (char*) os::malloc(newcap, mtLogging);
+    if (newbuf == NULL) { // OOM. Leave object unchanged.
+      return;
+    }
+    if (_pos > 0) { // preserve old content
+      memcpy(newbuf, _buf, _pos + 1); // ..including trailing zero
+    }
+    if (_buf != _smallbuf) {
+      os::free(_buf);
+    }
+    _buf = newbuf;
+    _cap = newcap;
+  }
+  assert(_cap >= atleast, "sanity");
+}

with the above code, even though _cap > reasonable_max, you will always 
allocate newbuf with os::malloc. For debug builds, we are fine, since 
the JVM will assert on _cap <= reasonable_max, but for product builds we 
are not. Would it be better to first check if _cap == reasonable_max? As 
in the following snippet:

void LogStream::LineBuffer::try_ensure_cap(size_t atleast) {
   if (_cap < atleast) {
     const size_t reasonable_max = 1 * M;
     if (_cap == reasonable_max) {
       // max memory used, "die" in debug builds, log in product builds
       log_warning(logging)("Max memory used for message: %s\n", _buf);
       ShouldNotReachHere();
       return;
     }

     size_t newcap = align_size_up(atleast + 64, 64);
     if (newcap > reasonable_max) {
       newcap = reasonable_max;
     }
     char* const newbuf = (char*) os::malloc(newcap, mtLogging);
     // rest of logic
   }
}

Thanks,
Erik