Merge topic 'cmuvstreamread-raii'

27be5ccd45 cmUVStreamRead: Return RAII handle to avoid memory leak

Acked-by: Kitware Robot <[email protected]>
Acked-by: buildbot <[email protected]>
Merge-request: !8749

Source/cmUVStream.h

@@ -3,8 +3,11 @@
 #pragma once
 
 #include <cassert>
+#include <functional>
 #include <istream>
 
+#include <cm/memory>
+
 #include <cm3p/uv.h>
 
 #include "cmUVHandlePtr.h"
@@ -104,28 +107,38 @@ void cmBasicUVPipeIStream<CharT, Traits>::close()
 
 using cmUVPipeIStream = cmBasicUVPipeIStream<char>;
 
+class cmUVStreamReadHandle
+{
+private:
+  std::vector<char> Buffer;
+  std::function<void(std::vector<char>)> OnRead;
+  std::function<void()> OnFinish;
+
+  template <typename ReadCallback, typename FinishCallback>
+  friend std::unique_ptr<cmUVStreamReadHandle> cmUVStreamRead(
+    uv_stream_t* stream, ReadCallback onRead, FinishCallback onFinish);
+};
+
 template <typename ReadCallback, typename FinishCallback>
-void cmUVStreamRead(uv_stream_t* stream, ReadCallback onRead,
-                    FinishCallback onFinish)
+std::unique_ptr<cmUVStreamReadHandle> cmUVStreamRead(uv_stream_t* stream,
+                                                     ReadCallback onRead,
+                                                     FinishCallback onFinish)
 {
-  struct ReadData
-  {
-    std::vector<char> Buffer;
-    ReadCallback OnRead;
-    FinishCallback OnFinish;
-  };
-
-  stream->data = new ReadData{ {}, std::move(onRead), std::move(onFinish) };
+  auto handle = cm::make_unique<cmUVStreamReadHandle>();
+  handle->OnRead = std::move(onRead);
+  handle->OnFinish = std::move(onFinish);
+
+  stream->data = handle.get();
   uv_read_start(
     stream,
     [](uv_handle_t* s, std::size_t suggestedSize, uv_buf_t* buffer) {
-      auto* data = static_cast<ReadData*>(s->data);
+      auto* data = static_cast<cmUVStreamReadHandle*>(s->data);
       data->Buffer.resize(suggestedSize);
       buffer->base = data->Buffer.data();
       buffer->len = suggestedSize;
     },
     [](uv_stream_t* s, ssize_t nread, const uv_buf_t* buffer) {
-      auto* data = static_cast<ReadData*>(s->data);
+      auto* data = static_cast<cmUVStreamReadHandle*>(s->data);
       if (nread > 0) {
         (void)buffer;
         assert(buffer->base == data->Buffer.data());
@@ -134,7 +147,8 @@ void cmUVStreamRead(uv_stream_t* stream, ReadCallback onRead,
       } else if (nread < 0 /*|| nread == UV_EOF*/) {
         data->OnFinish();
         uv_read_stop(s);
-        delete data;
       }
     });
+
+  return handle;
 }

Tests/CMakeLib/testUVStreambuf.cxx

@@ -503,7 +503,7 @@ bool testUVStreamRead()
 
   std::string output;
   bool finished = false;
-  cmUVStreamRead(
+  auto handle = cmUVStreamRead(
     pipeSource,
     [&output](std::vector<char> data) { cm::append(output, data); },
     [&output, &finished]() {
@@ -524,6 +524,55 @@ bool testUVStreamRead()
   return true;
 }
 
+bool testUVStreamReadLeak()
+{
+  int pipe[] = { -1, -1 };
+  if (cmGetPipes(pipe) < 0) {
+    std::cout << "cmGetPipes() returned an error" << std::endl;
+    return false;
+  }
+
+  cm::uv_loop_ptr loop;
+  loop.init();
+  cm::uv_pipe_ptr pipeSink;
+  pipeSink.init(*loop, 0);
+  uv_pipe_open(pipeSink, pipe[1]);
+
+  std::string str = "Hello world!";
+  uv_write_t writeReq;
+  uv_buf_t buf;
+  buf.base = &str.front();
+  buf.len = str.length();
+  uv_write(&writeReq, pipeSink, &buf, 1, nullptr);
+  uv_run(loop, UV_RUN_DEFAULT);
+  pipeSink.reset();
+
+  cm::uv_pipe_ptr pipeSource;
+  pipeSource.init(*loop, 0);
+  uv_pipe_open(pipeSource, pipe[0]);
+
+  std::string output;
+  bool finished = false;
+  auto handle = cmUVStreamRead(
+    pipeSource,
+    [&output](std::vector<char> data) { cm::append(output, data); },
+    [&output, &finished]() {
+      if (output != "Hello world!") {
+        std::cout << "Output was \"" << output
+                  << "\", should be \"Hello world!\"" << std::endl;
+        return;
+      }
+      finished = true;
+    });
+
+  if (finished) {
+    std::cout << "finished was set" << std::endl;
+    return false;
+  }
+
+  return true;
+}
+
 int testUVStreambuf(int argc, char** const argv)
 {
   if (argc < 2) {
@@ -547,7 +596,12 @@ int testUVStreambuf(int argc, char** const argv)
   }
 
   if (!testUVStreamRead()) {
-    std::cout << "While executing testUVPipeIStream().\n";
+    std::cout << "While executing testUVStreamRead().\n";
+    return -1;
+  }
+
+  if (!testUVStreamReadLeak()) {
+    std::cout << "While executing testUVStreamReadLeak().\n";
     return -1;
   }