From ff646d60ebae6b2df1d4889d3b682c814d120d20 Mon Sep 17 00:00:00 2001 From: Varada M Date: Wed, 19 Mar 2025 14:34:51 +0000 Subject: [PATCH 1/7] AIX: Problem list serviceability/attach/AttachAPIv2/StreamingOutputTest.java --- test/hotspot/jtreg/ProblemList.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt index b866df3020add..341bccd0e3244 100644 --- a/test/hotspot/jtreg/ProblemList.txt +++ b/test/hotspot/jtreg/ProblemList.txt @@ -130,6 +130,8 @@ containers/docker/TestJcmdWithSideCar.java 8341518 linux-x64 # :hotspot_serviceability +serviceability/attach/AttachAPIv2/StreamingOutputTest.java 8352392 aix-ppc64 + serviceability/sa/sadebugd/DebugdConnectTest.java 8239062,8270326 macosx-x64,macosx-aarch64 serviceability/sa/TestRevPtrsForInvokeDynamic.java 8241235 generic-all From b483f2bfe208596ef1d2eda7c9787bb3bfbc64e8 Mon Sep 17 00:00:00 2001 From: Varada M Date: Thu, 20 Mar 2025 06:02:22 +0000 Subject: [PATCH 2/7] AIX: implement attach API v2 and streaming output --- src/hotspot/os/aix/attachListener_aix.cpp | 234 ++++-------------- .../sun/tools/attach/VirtualMachineImpl.java | 60 ++--- test/hotspot/jtreg/ProblemList.txt | 1 - 3 files changed, 78 insertions(+), 217 deletions(-) diff --git a/src/hotspot/os/aix/attachListener_aix.cpp b/src/hotspot/os/aix/attachListener_aix.cpp index 218ee04fdcc0e..85ed0d9a3ca45 100644 --- a/src/hotspot/os/aix/attachListener_aix.cpp +++ b/src/hotspot/os/aix/attachListener_aix.cpp @@ -73,16 +73,7 @@ class AixAttachListener: AllStatic { static bool _atexit_registered; - // reads a request from the given connected socket - static AixAttachOperation* 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) { @@ -107,25 +98,64 @@ class AixAttachListener: AllStatic { static void set_shutdown(bool shutdown) { _shutdown = shutdown; } static bool is_shutdown() { return _shutdown; } - // write the given buffer to a socket - static int write_fully(int s, char* buf, size_t len); - static AixAttachOperation* 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()) { + ::shutdown(_socket, 2); + ::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(n); + } + + // ReplyWriter + int write(const void* buffer, int size) override { + ssize_t n; + RESTARTABLE(::write(_socket, buffer, size), n); + return checked_cast(n); + } + + void flush() override { + } +}; + class AixAttachOperation: public AttachOperation { private: // the connection to the client - int _socket; + SocketChannel _socket_channel; public: - void complete(jint res, bufferedStream* st); + AixAttachOperation(int socket) : AttachOperation(), _socket_channel(socket) {} - void set_socket(int s) { _socket = s; } - int socket() const { return _socket; } + void complete(jint res, bufferedStream* st) override; - AixAttachOperation(char* name) : AttachOperation(name) { - set_socket(-1); + ReplyWriter* get_reply_writer() override { + return &_socket_channel; + } + + bool read_request() { + return _socket_channel.read_request(this, &_socket_channel); } }; @@ -137,34 +167,6 @@ bool AixAttachListener::_atexit_registered = false; // Shutdown marker to prevent accept blocking during clean-up volatile bool AixAttachListener::_shutdown = 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; - } -}; - // On AIX if sockets block until all data has been transmitted // successfully in some communication domains a socket "close" may // never complete. We have to take care that after the socket shutdown @@ -258,106 +260,6 @@ int AixAttachListener::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. -// -AixAttachOperation* AixAttachListener::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: - // 00000 - // where is the protocol version (1), is the command - // name ("load", "datadump", ...), and 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; - // Don't block on interrupts because this will - // hang in the clean-up when shutting down. - n = read(s, buf+off, left); - assert(n <= checked_cast(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 (int i=0; i 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; - } - - AixAttachOperation* op = new AixAttachOperation(name); - - for (int i=0; iset_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 Aix implementation there is only a single operation and clients @@ -402,9 +304,9 @@ AixAttachOperation* AixAttachListener::dequeue() { } // peer credential look okay so we read the request - AixAttachOperation* op = read_request(s); - if (op == nullptr) { - ::close(s); + AixAttachOperation* op = new AixAttachOperation(s); + if (!op->read_request()) { + delete op; continue; } else { return op; @@ -412,21 +314,6 @@ AixAttachOperation* AixAttachListener::dequeue() { } } -// write the given buffer to the socket -int AixAttachListener::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 @@ -436,24 +323,6 @@ int AixAttachListener::write_fully(int s, char* buf, size_t len) { // socket could be made non-blocking and a timeout could be used. void AixAttachOperation::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 = AixAttachListener::write_fully(this->socket(), msg, strlen(msg)); - - // write any result data - if (rc == 0) { - // Shutdown the socket in the cleanup function to enable more than - // one agent attach in a sequence (see comments to listener_cleanup()). - AixAttachListener::write_fully(this->socket(), (char*) st->base(), st->size()); - } - - // done - ::close(this->socket()); - delete this; } @@ -493,6 +362,7 @@ void AttachListener::vm_start() { } int AttachListener::pd_init() { + AttachListener::set_supported_version(ATTACH_API_V2); JavaThread* thread = JavaThread::current(); ThreadBlockInVM tbivm(thread); diff --git a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java index 6df51d04b0a0a..6ed7aaf77de45 100644 --- a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java @@ -47,6 +47,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { // Any changes to this needs to be synchronized with HotSpot. private static final String tmpdir = "/tmp"; String socket_path; + private OperationProperties props = new OperationProperties(VERSION_1); /** * Attaches to the target VM @@ -110,11 +111,18 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { // Check that we can connect to the process // - this ensures we throw the permission denied error now rather than // later when we attempt to enqueue a command. - int s = socket(); - try { - connect(s, socket_path); - } finally { - close(s); + if (isAPIv2Enabled()) { + props = getDefaultProps(); + } else { + // Check that we can connect to the process + // - this ensures we throw the permission denied error now rather than + // later when we attempt to enqueue a command. + int s = socket(); + try { + connect(s, socket_path); + } finally { + close(s); + } } } @@ -129,14 +137,10 @@ public void detach() throws IOException { } } - // protocol version - private static final String PROTOCOL_VERSION = "1"; - /** * Execute the given command in the target VM. */ InputStream execute(String cmd, Object ... args) throws AgentLoadException, IOException { - assert args.length <= 3; // includes null checkNulls(args); // did we detach? @@ -162,16 +166,8 @@ InputStream execute(String cmd, Object ... args) throws AgentLoadException, IOEx // connected - write request // try { - writeString(s, PROTOCOL_VERSION); - writeString(s, cmd); - - for (int i = 0; i < 3; i++) { - if (i < args.length && args[i] != null) { - writeString(s, (String)args[i]); - } else { - writeString(s, ""); - } - } + SocketOutputStream writer = new SocketOutputStream(s); + writeCommand(writer, props, cmd, args); } catch (IOException x) { ioe = x; } @@ -187,6 +183,17 @@ InputStream execute(String cmd, Object ... args) throws AgentLoadException, IOEx return sis; } + private static class SocketOutputStream implements AttachOutputStream { + private int fd; + public SocketOutputStream(int fd) { + this.fd = fd; + } + @Override + public void write(byte[] buffer, int offset, int length) throws IOException { + VirtualMachineImpl.write(fd, buffer, offset, length); + } + } + /* * InputStream for the socket connection to get target VM */ @@ -223,21 +230,6 @@ private File createAttachFile(int pid) throws IOException { return f; } - /* - * Write/sends the given to the target VM. String is transmitted in - * UTF-8 encoding. - */ - private void writeString(int fd, String s) throws IOException { - if (s.length() > 0) { - byte[] b = s.getBytes(UTF_8); - VirtualMachineImpl.write(fd, b, 0, b.length); - } - byte b[] = new byte[1]; - b[0] = 0; - write(fd, b, 0, 1); - } - - //-- native methods static native void sendQuitTo(int pid) throws IOException; diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt index 341bccd0e3244..674ec3bd57cd7 100644 --- a/test/hotspot/jtreg/ProblemList.txt +++ b/test/hotspot/jtreg/ProblemList.txt @@ -130,7 +130,6 @@ containers/docker/TestJcmdWithSideCar.java 8341518 linux-x64 # :hotspot_serviceability -serviceability/attach/AttachAPIv2/StreamingOutputTest.java 8352392 aix-ppc64 serviceability/sa/sadebugd/DebugdConnectTest.java 8239062,8270326 macosx-x64,macosx-aarch64 serviceability/sa/TestRevPtrsForInvokeDynamic.java 8241235 generic-all From d31b62b96bf78fdadcf8959e06ba580f43eb02c7 Mon Sep 17 00:00:00 2001 From: Varada M Date: Sun, 23 Mar 2025 14:07:48 +0000 Subject: [PATCH 3/7] AIX: implement attach API v2 and streaming output --- test/hotspot/jtreg/ProblemList.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt index 674ec3bd57cd7..b866df3020add 100644 --- a/test/hotspot/jtreg/ProblemList.txt +++ b/test/hotspot/jtreg/ProblemList.txt @@ -130,7 +130,6 @@ containers/docker/TestJcmdWithSideCar.java 8341518 linux-x64 # :hotspot_serviceability - serviceability/sa/sadebugd/DebugdConnectTest.java 8239062,8270326 macosx-x64,macosx-aarch64 serviceability/sa/TestRevPtrsForInvokeDynamic.java 8241235 generic-all From 8efd253f3d0a58233c0fde31a9514b8c911c892c Mon Sep 17 00:00:00 2001 From: Varada M Date: Sun, 23 Mar 2025 15:30:49 +0000 Subject: [PATCH 4/7] AIX: implement attach API v2 and streaming output --- .../aix/classes/sun/tools/attach/VirtualMachineImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java index 6ed7aaf77de45..ccc2efcf12264 100644 --- a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java @@ -166,7 +166,7 @@ InputStream execute(String cmd, Object ... args) throws AgentLoadException, IOEx // connected - write request // try { - SocketOutputStream writer = new SocketOutputStream(s); + SocketOutputStream writer = new SocketOutputStream(s); writeCommand(writer, props, cmd, args); } catch (IOException x) { ioe = x; From 90af6650d3af601c1e32345109f8ea325e6f35bb Mon Sep 17 00:00:00 2001 From: Varada M Date: Fri, 28 Mar 2025 08:33:02 +0000 Subject: [PATCH 5/7] removed StreamingOutputTest.java from problem list --- test/hotspot/jtreg/ProblemList.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt index 0e71df4f27fae..6dd35997caca3 100644 --- a/test/hotspot/jtreg/ProblemList.txt +++ b/test/hotspot/jtreg/ProblemList.txt @@ -128,8 +128,6 @@ containers/docker/TestJcmdWithSideCar.java 8341518 linux-x64 # :hotspot_serviceability -serviceability/attach/AttachAPIv2/StreamingOutputTest.java 8352392 aix-ppc64 - serviceability/sa/sadebugd/DebugdConnectTest.java 8239062,8270326 macosx-x64,macosx-aarch64 serviceability/sa/TestRevPtrsForInvokeDynamic.java 8241235 generic-all From d22f0ab957c9ab8101f184d21f822082fac9d3c3 Mon Sep 17 00:00:00 2001 From: Varada M Date: Fri, 28 Mar 2025 08:35:43 +0000 Subject: [PATCH 6/7] updated copyright header --- .../aix/classes/sun/tools/attach/VirtualMachineImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java index ccc2efcf12264..722dce4b9fdac 100644 --- a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 2025, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2015, 2019 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * From 234f6d17d24bd2aaea5ff0011e4d8dddd5352031 Mon Sep 17 00:00:00 2001 From: Varada M Date: Tue, 1 Apr 2025 10:04:16 +0000 Subject: [PATCH 7/7] 8352392: AIX: implement attach API v2 and streaming output --- src/hotspot/os/aix/attachListener_aix.cpp | 1 + .../sun/tools/attach/VirtualMachineImpl.java | 38 +++++++++---------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/hotspot/os/aix/attachListener_aix.cpp b/src/hotspot/os/aix/attachListener_aix.cpp index 85ed0d9a3ca45..e5101814f9771 100644 --- a/src/hotspot/os/aix/attachListener_aix.cpp +++ b/src/hotspot/os/aix/attachListener_aix.cpp @@ -116,6 +116,7 @@ class SocketChannel : public AttachOperation::RequestReader, public AttachOperat void close() { if (opened()) { + // SHUT_RDWR is not available ::shutdown(_socket, 2); ::close(_socket); _socket = -1; diff --git a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java index 722dce4b9fdac..f162ec48d2807 100644 --- a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java @@ -112,17 +112,17 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { // - this ensures we throw the permission denied error now rather than // later when we attempt to enqueue a command. if (isAPIv2Enabled()) { - props = getDefaultProps(); + props = getDefaultProps(); } else { - // Check that we can connect to the process - // - this ensures we throw the permission denied error now rather than - // later when we attempt to enqueue a command. - int s = socket(); - try { - connect(s, socket_path); - } finally { - close(s); - } + // Check that we can connect to the process + // - this ensures we throw the permission denied error now rather than + // later when we attempt to enqueue a command. + int s = socket(); + try { + connect(s, socket_path); + } finally { + close(s); + } } } @@ -184,15 +184,15 @@ InputStream execute(String cmd, Object ... args) throws AgentLoadException, IOEx } private static class SocketOutputStream implements AttachOutputStream { - private int fd; - public SocketOutputStream(int fd) { - this.fd = fd; - } - @Override - public void write(byte[] buffer, int offset, int length) throws IOException { - VirtualMachineImpl.write(fd, buffer, offset, length); - } - } + private int fd; + public SocketOutputStream(int fd) { + this.fd = fd; + } + @Override + public void write(byte[] buffer, int offset, int length) throws IOException { + VirtualMachineImpl.write(fd, buffer, offset, length); + } + } /* * InputStream for the socket connection to get target VM