8342995: Enhance Attach API to support arbitrary length arguments - Linux

Reviewed-by: sspitsyn, kevinw

Author: Alex Menkov

src/hotspot/os/posix/attachListener_posix.cpp

@@ -76,17 +76,7 @@ class PosixAttachListener: AllStatic {
 
   static bool _atexit_registered;
 
-  // reads a request from the given connected socket
-  static PosixAttachOperation* read_request(int s);
-
  public:
-  enum {
-    ATTACH_PROTOCOL_VER = 1                     // protocol version
-  };
-  enum {
-    ATTACH_ERROR_BADVERSION     = 101           // error codes
-  };
-
   static void set_path(char* path) {
     if (path == nullptr) {
       _path[0] = '\0';
@@ -107,25 +97,61 @@ class PosixAttachListener: AllStatic {
   static bool has_path()                { return _has_path; }
   static int listener()                 { return _listener; }
 
-  // write the given buffer to a socket
-  static int write_fully(int s, char* buf, size_t len);
-
   static PosixAttachOperation* dequeue();
 };
 
+class SocketChannel : public AttachOperation::RequestReader, public AttachOperation::ReplyWriter {
+private:
+  int _socket;
+public:
+  SocketChannel(int socket) : _socket(socket) {}
+  ~SocketChannel() {
+    close();
+  }
+
+  bool opened() const {
+    return _socket != -1;
+  }
+
+  void close() {
+    if (opened()) {
+      ::close(_socket);
+      _socket = -1;
+    }
+  }
+
+  // RequestReader
+  int read(void* buffer, int size) override {
+    ssize_t n;
+    RESTARTABLE(::read(_socket, buffer, (size_t)size), n);
+    return checked_cast<int>(n);
+  }
+
+  // ReplyWriter
+  int write(const void* buffer, int size) override {
+    ssize_t n;
+    RESTARTABLE(::write(_socket, buffer, size), n);
+    return checked_cast<int>(n);
+  }
+  // called after writing all data
+  void flush() override {
+    ::shutdown(_socket, SHUT_RDWR);
+  }
+};
+
 class PosixAttachOperation: public AttachOperation {
  private:
   // the connection to the client
-  int _socket;
+  SocketChannel _socket_channel;
 
  public:
-  void complete(jint res, bufferedStream* st);
+  void complete(jint res, bufferedStream* st) override;
 
-  void set_socket(int s)                                { _socket = s; }
-  int socket() const                                    { return _socket; }
+  PosixAttachOperation(int socket) : AttachOperation(), _socket_channel(socket) {
+  }
 
-  PosixAttachOperation(char* name) : AttachOperation(name) {
-    set_socket(-1);
+  bool read_request() {
+    return AttachOperation::read_request(&_socket_channel, &_socket_channel);
   }
 };
 
@@ -135,35 +161,6 @@ bool PosixAttachListener::_has_path;
 volatile int PosixAttachListener::_listener = -1;
 bool PosixAttachListener::_atexit_registered = false;
 
-// Supporting class to help split a buffer into individual components
-class ArgumentIterator : public StackObj {
- private:
-  char* _pos;
-  char* _end;
- public:
-  ArgumentIterator(char* arg_buffer, size_t arg_size) {
-    _pos = arg_buffer;
-    _end = _pos + arg_size - 1;
-  }
-  char* next() {
-    if (*_pos == '\0') {
-      // advance the iterator if possible (null arguments)
-      if (_pos < _end) {
-        _pos += 1;
-      }
-      return nullptr;
-    }
-    char* res = _pos;
-    char* next_pos = strchr(_pos, '\0');
-    if (next_pos < _end)  {
-      next_pos++;
-    }
-    _pos = next_pos;
-    return res;
-  }
-};
-
-
 // atexit hook to stop listener and unlink the file that it is
 // bound too.
 extern "C" {
@@ -249,103 +246,6 @@ int PosixAttachListener::init() {
   return 0;
 }
 
-// Given a socket that is connected to a peer we read the request and
-// create an AttachOperation. As the socket is blocking there is potential
-// for a denial-of-service if the peer does not response. However this happens
-// after the peer credentials have been checked and in the worst case it just
-// means that the attach listener thread is blocked.
-//
-PosixAttachOperation* PosixAttachListener::read_request(int s) {
-  char ver_str[8];
-  os::snprintf_checked(ver_str, sizeof(ver_str), "%d", ATTACH_PROTOCOL_VER);
-
-  // The request is a sequence of strings so we first figure out the
-  // expected count and the maximum possible length of the request.
-  // The request is:
-  //   <ver>0<cmd>0<arg>0<arg>0<arg>0
-  // where <ver> is the protocol version (1), <cmd> is the command
-  // name ("load", "datadump", ...), and <arg> is an argument
-  int expected_str_count = 2 + AttachOperation::arg_count_max;
-  const size_t max_len = (sizeof(ver_str) + 1) + (AttachOperation::name_length_max + 1) +
-    AttachOperation::arg_count_max*(AttachOperation::arg_length_max + 1);
-
-  char buf[max_len];
-  int str_count = 0;
-
-  // Read until all (expected) strings have been read, the buffer is
-  // full, or EOF.
-
-  size_t off = 0;
-  size_t left = max_len;
-
-  do {
-    ssize_t n;
-    RESTARTABLE(read(s, buf+off, left), n);
-    assert(n <= checked_cast<ssize_t>(left), "buffer was too small, impossible!");
-    buf[max_len - 1] = '\0';
-    if (n == -1) {
-      return nullptr;      // reset by peer or other error
-    }
-    if (n == 0) {
-      break;
-    }
-    for (ssize_t i=0; i<n; i++) {
-      if (buf[off+i] == 0) {
-        // EOS found
-        str_count++;
-
-        // The first string is <ver> so check it now to
-        // check for protocol mismatch
-        if (str_count == 1) {
-          if ((strlen(buf) != strlen(ver_str)) ||
-              (atoi(buf) != ATTACH_PROTOCOL_VER)) {
-            char msg[32];
-            os::snprintf_checked(msg, sizeof(msg), "%d\n", ATTACH_ERROR_BADVERSION);
-            write_fully(s, msg, strlen(msg));
-            return nullptr;
-          }
-        }
-      }
-    }
-    off += n;
-    left -= n;
-  } while (left > 0 && str_count < expected_str_count);
-
-  if (str_count != expected_str_count) {
-    return nullptr;        // incomplete request
-  }
-
-  // parse request
-
-  ArgumentIterator args(buf, (max_len)-left);
-
-  // version already checked
-  char* v = args.next();
-
-  char* name = args.next();
-  if (name == nullptr || strlen(name) > AttachOperation::name_length_max) {
-    return nullptr;
-  }
-
-  PosixAttachOperation* op = new PosixAttachOperation(name);
-
-  for (int i=0; i<AttachOperation::arg_count_max; i++) {
-    char* arg = args.next();
-    if (arg == nullptr) {
-      op->set_arg(i, nullptr);
-    } else {
-      if (strlen(arg) > AttachOperation::arg_length_max) {
-        delete op;
-        return nullptr;
-      }
-      op->set_arg(i, arg);
-    }
-  }
-
-  op->set_socket(s);
-  return op;
-}
-
 // Dequeue an operation
 //
 // In the Linux and BSD implementations, there is only a single operation and
@@ -400,31 +300,16 @@ PosixAttachOperation* PosixAttachListener::dequeue() {
 #endif
 
     // peer credential look okay so we read the request
-    PosixAttachOperation* op = read_request(s);
-    if (op == nullptr) {
-      ::close(s);
+    PosixAttachOperation* op = new PosixAttachOperation(s);
+    if (!op->read_request()) {
+      delete op;
       continue;
     } else {
       return op;
     }
   }
 }
 
-// write the given buffer to the socket
-int PosixAttachListener::write_fully(int s, char* buf, size_t len) {
-  do {
-    ssize_t n = ::write(s, buf, len);
-    if (n == -1) {
-      if (errno != EINTR) return -1;
-    } else {
-      buf += n;
-      len -= n;
-    }
-  }
-  while (len > 0);
-  return 0;
-}
-
 // Complete an operation by sending the operation result and any result
 // output to the client. At this time the socket is in blocking mode so
 // potentially we can block if there is a lot of data and the client is
@@ -437,19 +322,7 @@ void PosixAttachOperation::complete(jint result, bufferedStream* st) {
   JavaThread* thread = JavaThread::current();
   ThreadBlockInVM tbivm(thread);
 
-  // write operation result
-  char msg[32];
-  os::snprintf_checked(msg, sizeof(msg), "%d\n", result);
-  int rc = PosixAttachListener::write_fully(this->socket(), msg, strlen(msg));
-
-  // write any result data
-  if (rc == 0) {
-    PosixAttachListener::write_fully(this->socket(), (char*) st->base(), st->size());
-    ::shutdown(this->socket(), 2);
-  }
-
-  // done
-  ::close(this->socket());
+  write_reply(&_socket_channel, result, st);
 
   delete this;
 }
@@ -490,6 +363,8 @@ void AttachListener::vm_start() {
 }
 
 int AttachListener::pd_init() {
+  AttachListener::set_supported_version(ATTACH_API_V2);
+
   JavaThread* thread = JavaThread::current();
   ThreadBlockInVM tbivm(thread);
 

src/hotspot/os/windows/attachListener_windows.cpp

@@ -152,7 +152,7 @@ class Win32AttachOperation: public AttachOperation {
   }
 
   bool read_request() {
-    return AttachOperation::read_request(&_pipe);
+    return AttachOperation::read_request(&_pipe, &_pipe);
   }
 
 public: