kparzysz-quic commented on code in PR #10967:
URL: https://github.com/apache/tvm/pull/10967#discussion_r852412804


##########
src/runtime/minrpc/minrpc_server.h:
##########
@@ -524,94 +523,249 @@ class MinRPCServer {
     int call_ecode = TVMSetStream(dev.device_type, dev.device_id, handle);
 
     if (call_ecode == 0) {
-      this->ReturnVoid();
+      ret_handler_->ReturnVoid();
     } else {
-      this->ReturnLastTVMError();
+      ret_handler_->ReturnLastTVMError();
     }
   }
 
   void ThrowError(RPCServerStatus code, RPCCode info = RPCCode::kNone) {
-    io_->Exit(static_cast<int>(code));
+    ret_handler_->ThrowError(code, info);
   }
 
+  ReturnInterface* GetReturnInterface() { return ret_handler_; }
+
+ private:
   template <typename T>
-  T* ArenaAlloc(int count) {
-    static_assert(std::is_pod<T>::value, "need to be trival");
-    return arena_.template allocate_<T>(count);
+  int ReadArray(T* data, size_t count) {
+    static_assert(std::is_trivial<T>::value && 
std::is_standard_layout<T>::value,
+                  "need to be trival");
+    return ReadRawBytes(data, sizeof(T) * count);
   }
 
-  template <typename T>
-  void Read(T* data) {
-    static_assert(std::is_pod<T>::value, "need to be trival");
-    this->ReadRawBytes(data, sizeof(T));
+  int ReadRawBytes(void* data, size_t size) {
+    uint8_t* buf = reinterpret_cast<uint8_t*>(data);
+    size_t ndone = 0;
+    while (ndone < size) {
+      ssize_t ret = io_->PosixRead(buf, size - ndone);
+      if (ret <= 0) return ret;
+      ndone += ret;
+      buf += ret;
+    }
+    return 1;
   }
 
-  template <typename T>
-  void ReadArray(T* data, size_t count) {
-    static_assert(std::is_pod<T>::value, "need to be trival");
-    return this->ReadRawBytes(data, sizeof(T) * count);
+  TIOHandler* io_;
+  ReturnInterface* ret_handler_;
+};
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ *  All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ *         An IOHandler needs to provide the following functions:
+ *         - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ *         - MessageStart(num_bytes), MessageDone(): framing APIs.
+ *         - Exit: exit with status code.
+ */
+template <typename TIOHandler, template <typename> class Allocator = 
detail::PageAllocator>
+class MinRPCServer {
+ public:
+  using PageAllocator = Allocator<TIOHandler>;
+
+  /*!
+   * \brief Constructor.
+   * \param io The IO handler.
+   */
+  MinRPCServer(TIOHandler* io, ExecInterface* exec_handler)
+      : io_(io), arena_(PageAllocator(io_)), exec_handler_(exec_handler) {}
+
+  explicit MinRPCServer(TIOHandler* io)
+      : io_(io),
+        arena_(PageAllocator(io)),
+        ret_handler_(new MinRPCReturns<TIOHandler>(io_)),
+        exec_handler_(new MinRPCExecute<TIOHandler>(io_, ret_handler_)) {}

Review Comment:
   These two members are allocated, but never deleted.  In the logging classes 
you pass addresses of members to the `MinRPCServer` constructor, so it doesn't 
have a way of knowing whether it should delete these objects or not.  This 
needs to be fixed or it will cause memory leaks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to