From fbc1c379e19a82d19912209ee0e389f1bf546ef2 Mon Sep 17 00:00:00 2001 From: Alex Menkov Date: Fri, 31 Jan 2025 15:18:35 -0800 Subject: [PATCH 1/3] fix --- src/hotspot/os/posix/attachListener_posix.cpp | 17 +- .../os/windows/attachListener_windows.cpp | 18 +- src/hotspot/share/services/attachListener.cpp | 415 ++++++++++++++---- src/hotspot/share/services/attachListener.hpp | 72 ++- .../share/services/diagnosticFramework.cpp | 18 +- .../share/services/diagnosticFramework.hpp | 15 +- .../sun/tools/attach/VirtualMachineImpl.java | 8 +- .../sun/tools/attach/VirtualMachineImpl.java | 7 +- .../tools/attach/HotSpotVirtualMachine.java | 100 ++++- .../sun/tools/attach/VirtualMachineImpl.java | 20 +- .../AttachAPIv2/StreamingOutputTest.java | 93 ++++ .../compiler/CodeHeapAnalyticsParams.java | 8 +- 12 files changed, 612 insertions(+), 179 deletions(-) create mode 100644 test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java diff --git a/src/hotspot/os/posix/attachListener_posix.cpp b/src/hotspot/os/posix/attachListener_posix.cpp index 27728d0ca1f6e..375e5e5e6e033 100644 --- a/src/hotspot/os/posix/attachListener_posix.cpp +++ b/src/hotspot/os/posix/attachListener_posix.cpp @@ -114,6 +114,7 @@ class SocketChannel : public AttachOperation::RequestReader, public AttachOperat void close() { if (opened()) { + ::shutdown(_socket, SHUT_RDWR); ::close(_socket); _socket = -1; } @@ -132,9 +133,8 @@ class SocketChannel : public AttachOperation::RequestReader, public AttachOperat RESTARTABLE(::write(_socket, buffer, size), n); return checked_cast(n); } - // called after writing all data + void flush() override { - ::shutdown(_socket, SHUT_RDWR); } }; @@ -144,13 +144,17 @@ class PosixAttachOperation: public AttachOperation { SocketChannel _socket_channel; public: + PosixAttachOperation(int socket) : AttachOperation(), _socket_channel(socket) { + } + void complete(jint res, bufferedStream* st) override; - PosixAttachOperation(int socket) : AttachOperation(), _socket_channel(socket) { + virtual ReplyWriter* get_reply_writer() override { + return &_socket_channel; } bool read_request() { - return AttachOperation::read_request(&_socket_channel, &_socket_channel); + return _socket_channel.read_request(this, &_socket_channel); } }; @@ -318,11 +322,6 @@ PosixAttachOperation* PosixAttachListener::dequeue() { // socket could be made non-blocking and a timeout could be used. void PosixAttachOperation::complete(jint result, bufferedStream* st) { - JavaThread* thread = JavaThread::current(); - ThreadBlockInVM tbivm(thread); - - write_reply(&_socket_channel, result, st); - delete this; } diff --git a/src/hotspot/os/windows/attachListener_windows.cpp b/src/hotspot/os/windows/attachListener_windows.cpp index 4e6f39b8f8189..4d6c4d5e9554e 100644 --- a/src/hotspot/os/windows/attachListener_windows.cpp +++ b/src/hotspot/os/windows/attachListener_windows.cpp @@ -92,6 +92,7 @@ class PipeChannel : public AttachOperation::RequestReader, public AttachOperatio void close() { if (opened()) { + FlushFileBuffers(_hPipe); CloseHandle(_hPipe); _hPipe = INVALID_HANDLE_VALUE; } @@ -123,15 +124,13 @@ class PipeChannel : public AttachOperation::RequestReader, public AttachOperatio &written, nullptr); // not overlapped if (!fSuccess) { - log_error(attach)("pipe write error (%d)", GetLastError()); - return -1; + log_error(attach)("pipe write error (%d)", GetLastError()); + return -1; } return (int)written; } void flush() override { - assert(opened(), "must be"); - FlushFileBuffers(_hPipe); } }; @@ -151,11 +150,15 @@ class Win32AttachOperation: public AttachOperation { } bool read_request() { - return AttachOperation::read_request(&_pipe, &_pipe); + return _pipe.read_request(this, &_pipe); } public: void complete(jint result, bufferedStream* result_stream) override; + + virtual ReplyWriter* get_reply_writer() override { + return &_pipe; + } }; @@ -432,11 +435,6 @@ Win32AttachOperation* Win32AttachListener::dequeue() { } void Win32AttachOperation::complete(jint result, bufferedStream* result_stream) { - JavaThread* thread = JavaThread::current(); - ThreadBlockInVM tbivm(thread); - - write_reply(&_pipe, result, result_stream); - delete this; } diff --git a/src/hotspot/share/services/attachListener.cpp b/src/hotspot/share/services/attachListener.cpp index d08965c0b2018..717d54773c9b7 100644 --- a/src/hotspot/share/services/attachListener.cpp +++ b/src/hotspot/share/services/attachListener.cpp @@ -36,6 +36,7 @@ #include "runtime/flags/jvmFlag.hpp" #include "runtime/globals.hpp" #include "runtime/handles.inline.hpp" +#include "runtime/interfaceSupport.inline.hpp" #include "runtime/java.hpp" #include "runtime/javaCalls.hpp" #include "runtime/os.hpp" @@ -47,10 +48,128 @@ #include "utilities/debug.hpp" #include "utilities/formatBuffer.hpp" + +// Stream for printing attach operation results. +// Supports buffered and streaming output for commands which can produce lenghtly reply. +// +// To support streaming output platform implementation need to implement AttachOperation::get_reply_writer() method +// and ctor allow_streaming argument should be set to true. +// +// Initially attachStream works in buffered mode. +// To switch to the streaming mode attach command handler need to call attachStream::set_result(). +// The method flushes buffered output and consequent printing goes directly to ReplyWriter. +class attachStream : public bufferedStream { + AttachOperation::ReplyWriter* _reply_writer; + bool _allow_streaming; + jint _result; + bool _result_set; + bool _result_written; + bool _error; + + enum : size_t { + INITIAL_BUFFER_SIZE = 1 * M, + MAXIMUM_BUFFER_SIZE = 3 * G, + }; + + bool is_streaming() const { + return _result_set && _allow_streaming; + } + + void flush_reply() { + if (_error) { + return; + } + + if (!_result_written) { + if (_reply_writer->write_reply(_result, this)) { + _result_written = true; + reset(); + } else { + _error = true; + return; + } + } else { + _error = !_reply_writer->write_fully(base(), (int)size()); + reset(); + } + } + +public: + attachStream(AttachOperation::ReplyWriter* reply_writer, bool allow_streaming) + : bufferedStream(INITIAL_BUFFER_SIZE, MAXIMUM_BUFFER_SIZE), + _reply_writer(reply_writer), + _allow_streaming(reply_writer == nullptr ? false : allow_streaming), + _result(JNI_OK), _result_set(false), _result_written(false), + _error(false) + {} + + virtual ~attachStream() {} + + void set_result(jint result) { + if (!_result_set) { + _result = result; + _result_set = true; + if (_allow_streaming) { + // switch to streaming mode + flush_reply(); + } + } + } + + jint get_result() const { + return _result; + } + + bufferedStream* get_buffered_stream() { + return this; + } + + // Called after the operation is completed. + // If reply_writer is provided, writes the results. + void complete() { + flush_reply(); + } + + virtual void write(const char* str, size_t len) override { + if (is_streaming()) { + if (!_error) { + _error = !_reply_writer->write_fully(str, (int)len); + } + } else { + /* TODO: handle buffer overflow + if (size() + len > MAXIMUM_BUFFER_SIZE) { + } + */ + bufferedStream::write(str, len); + } + } + + virtual void flush() override { + // flush if streaming output is enabled + if (_allow_streaming) { + flush_reply(); + } else { + bufferedStream::flush(); + } + } +}; + +// Attach operation handler. +typedef void(*AttachOperationFunction)(AttachOperation* op, attachStream* out); + +struct AttachOperationFunctionInfo { + const char* name; + AttachOperationFunction func; +}; + + volatile AttachListenerState AttachListener::_state = AL_NOT_INITIALIZED; AttachAPIVersion AttachListener::_supported_version = ATTACH_API_V1; +// Default is false (if jdk.attach.vm.streaming property is not set). +bool AttachListener::_default_streaming_output = false; + static bool get_bool_sys_prop(const char* name, bool default_value, TRAPS) { ResourceMark rm(THREAD); HandleMark hm(THREAD); @@ -95,7 +214,7 @@ static InstanceKlass* load_and_initialize_klass(Symbol* sh, TRAPS) { return ik; } -static jint get_properties(AttachOperation* op, outputStream* out, Symbol* serializePropertiesMethod) { +static void get_properties(AttachOperation* op, attachStream* out, Symbol* serializePropertiesMethod) { JavaThread* THREAD = JavaThread::current(); // For exception macros. HandleMark hm(THREAD); @@ -105,7 +224,8 @@ static jint get_properties(AttachOperation* op, outputStream* out, Symbol* seria if (HAS_PENDING_EXCEPTION) { java_lang_Throwable::print(PENDING_EXCEPTION, out); CLEAR_PENDING_EXCEPTION; - return JNI_ERR; + out->set_result(JNI_ERR); + return; } // invoke the serializePropertiesToByteArray method @@ -123,7 +243,8 @@ static jint get_properties(AttachOperation* op, outputStream* out, Symbol* seria if (HAS_PENDING_EXCEPTION) { java_lang_Throwable::print(PENDING_EXCEPTION, out); CLEAR_PENDING_EXCEPTION; - return JNI_ERR; + out->set_result(JNI_ERR); + return; } // The result should be a [B @@ -131,16 +252,16 @@ static jint get_properties(AttachOperation* op, outputStream* out, Symbol* seria assert(res->is_typeArray(), "just checking"); assert(TypeArrayKlass::cast(res->klass())->element_type() == T_BYTE, "just checking"); + out->set_result(JNI_OK); + // copy the bytes to the output stream typeArrayOop ba = typeArrayOop(res); jbyte* addr = typeArrayOop(res)->byte_at_addr(0); out->print_raw((const char*)addr, ba->length()); - - return JNI_OK; } // Implementation of "load" command. -static jint load_agent(AttachOperation* op, outputStream* out) { +static void load_agent(AttachOperation* op, attachStream* out) { // get agent name and options const char* agent = op->arg(0); const char* absParam = op->arg(1); @@ -160,29 +281,30 @@ static jint load_agent(AttachOperation* op, outputStream* out) { h_module_name, THREAD); if (HAS_PENDING_EXCEPTION) { + out->set_result(JNI_ERR); java_lang_Throwable::print(PENDING_EXCEPTION, out); CLEAR_PENDING_EXCEPTION; - return JNI_ERR; + return; } } + out->set_result(JNI_OK); // The abs parameter should be "true" or "false". const bool is_absolute_path = (absParam != nullptr) && (strcmp(absParam, "true") == 0); JvmtiAgentList::load_agent(agent, is_absolute_path, options, out); // Agent_OnAttach result or error message is written to 'out'. - return JNI_OK; } // Implementation of "properties" command. // See also: PrintSystemPropertiesDCmd class -static jint get_system_properties(AttachOperation* op, outputStream* out) { +static void get_system_properties(AttachOperation* op, attachStream* out) { return get_properties(op, out, vmSymbols::serializePropertiesToByteArray_name()); } // Implementation of "agent_properties" command. -static jint get_agent_properties(AttachOperation* op, outputStream* out) { - return get_properties(op, out, vmSymbols::serializeAgentPropertiesToByteArray_name()); +static void get_agent_properties(AttachOperation* op, attachStream* out) { + get_properties(op, out, vmSymbols::serializeAgentPropertiesToByteArray_name()); } // Implementation of "datadump" command. @@ -192,7 +314,8 @@ static jint get_agent_properties(AttachOperation* op, outputStream* out) { // JVMTI environment that has enabled this event. However it's useful to // trigger the SIGBREAK handler. -static jint data_dump(AttachOperation* op, outputStream* out) { +static void data_dump(AttachOperation* op, attachStream* out) { + out->set_result(JNI_OK); if (!ReduceSignalUsage) { AttachListener::pd_data_dump(); } else { @@ -200,13 +323,12 @@ static jint data_dump(AttachOperation* op, outputStream* out) { JvmtiExport::post_data_dump(); } } - return JNI_OK; } // Implementation of "threaddump" command - essentially a remote ctrl-break // See also: ThreadDumpDCmd class // -static jint thread_dump(AttachOperation* op, outputStream* out) { +static void thread_dump(AttachOperation* op, attachStream* out) { bool print_concurrent_locks = false; bool print_extended_info = false; if (op->arg(0) != nullptr) { @@ -220,6 +342,8 @@ static jint thread_dump(AttachOperation* op, outputStream* out) { } } + out->set_result(JNI_OK); + // thread stacks and JNI global handles VM_PrintThreads op1(out, print_concurrent_locks, print_extended_info, true /* print JNI handle info */); VMThread::execute(&op1); @@ -227,24 +351,55 @@ static jint thread_dump(AttachOperation* op, outputStream* out) { // Deadlock detection VM_FindDeadlocks op2(out); VMThread::execute(&op2); - - return JNI_OK; } // A jcmd attach operation request was received, which will now // dispatch to the diagnostic commands used for serviceability functions. -static jint jcmd(AttachOperation* op, outputStream* out) { +static void jcmd(AttachOperation* op, attachStream* out) { JavaThread* THREAD = JavaThread::current(); // For exception macros. + // All the supplied jcmd arguments are stored as a single // string (op->arg(0)). This is parsed by the Dcmd framework. - DCmd::parse_and_execute(DCmd_Source_AttachAPI, out, op->arg(0), ' ', THREAD); + + // Overridden DCmd::Executor to switch output to "streaming" mode + // before execute the command. + + bool allow_streaming_output = true; + // Special case for ManagementAgent.start and ManagementAgent.start_local commands + // used by HotSpotVirtualMachine.startManagementAgent and startLocalManagementAgent. + // The commands report error if the agent failed to load, so we need to disable streaming output. + const char jmx_prefix[] = "ManagementAgent."; + if (strncmp(op->arg(0), jmx_prefix, strlen(jmx_prefix)) == 0) { + allow_streaming_output = false; + } + + class Executor: public DCmd::Executor { + private: + attachStream* _attach_stream; + bool _allow_streaming_output; + public: + Executor(DCmdSource source, attachStream* out, bool allow_streaming_output) + : DCmd::Executor(source, out), _attach_stream(out), _allow_streaming_output(allow_streaming_output) {} + protected: + virtual void execute(DCmd* command, TRAPS) override { + if (_allow_streaming_output) { + _attach_stream->set_result(JNI_OK); + } + DCmd::Executor::execute(command, CHECK); + } + } executor(DCmd_Source_AttachAPI, out, allow_streaming_output); + + executor.parse_and_execute(op->arg(0), ' ', THREAD); if (HAS_PENDING_EXCEPTION) { + // We can get an exception during command execution. + // In the case _attach_stream->set_result() is already called and operation result code is send + // to the client. + // Repeated out->set_result() is a no-op, just report exception message. + out->set_result(JNI_ERR); java_lang_Throwable::print(PENDING_EXCEPTION, out); out->cr(); CLEAR_PENDING_EXCEPTION; - return JNI_ERR; } - return JNI_OK; } // Implementation of "dumpheap" command. @@ -254,40 +409,45 @@ static jint jcmd(AttachOperation* op, outputStream* out) { // arg0: Name of the dump file // arg1: "-live" or "-all" // arg2: Compress level -static jint dump_heap(AttachOperation* op, outputStream* out) { +static void dump_heap(AttachOperation* op, attachStream* out) { const char* path = op->arg(0); if (path == nullptr || path[0] == '\0') { + out->set_result(JNI_ERR); out->print_cr("No dump file specified"); - } else { - bool live_objects_only = true; // default is true to retain the behavior before this change is made - const char* arg1 = op->arg(1); - if (arg1 != nullptr && (strlen(arg1) > 0)) { - if (strcmp(arg1, "-all") != 0 && strcmp(arg1, "-live") != 0) { - out->print_cr("Invalid argument to dumpheap operation: %s", arg1); - return JNI_ERR; - } - live_objects_only = strcmp(arg1, "-live") == 0; - } - - const char* num_str = op->arg(2); - uint level = 0; - if (num_str != nullptr && num_str[0] != '\0') { - if (!Arguments::parse_uint(num_str, &level, 0)) { - out->print_cr("Invalid compress level: [%s]", num_str); - return JNI_ERR; - } else if (level < 1 || level > 9) { - out->print_cr("Compression level out of range (1-9): %u", level); - return JNI_ERR; - } + return; + } + + bool live_objects_only = true; // default is true to retain the behavior before this change is made + const char* arg1 = op->arg(1); + if (arg1 != nullptr && (strlen(arg1) > 0)) { + if (strcmp(arg1, "-all") != 0 && strcmp(arg1, "-live") != 0) { + out->set_result(JNI_ERR); + out->print_cr("Invalid argument to dumpheap operation: %s", arg1); + return; } + live_objects_only = strcmp(arg1, "-live") == 0; + } - // Request a full GC before heap dump if live_objects_only = true - // This helps reduces the amount of unreachable objects in the dump - // and makes it easier to browse. - HeapDumper dumper(live_objects_only /* request GC */); - dumper.dump(path, out, level); + const char* num_str = op->arg(2); + uint level = 0; + if (num_str != nullptr && num_str[0] != '\0') { + if (!Arguments::parse_uint(num_str, &level, 0)) { + out->set_result(JNI_ERR); + out->print_cr("Invalid compress level: [%s]", num_str); + return; + } else if (level < 1 || level > 9) { + out->set_result(JNI_ERR); + out->print_cr("Compression level out of range (1-9): %u", level); + return; + } } - return JNI_OK; + + out->set_result(JNI_OK); + // Request a full GC before heap dump if live_objects_only = true + // This helps reduces the amount of unreachable objects in the dump + // and makes it easier to browse. + HeapDumper dumper(live_objects_only /* request GC */); + dumper.dump(path, out, level); } // Implementation of "inspectheap" command @@ -297,7 +457,7 @@ static jint dump_heap(AttachOperation* op, outputStream* out) { // arg0: "-live" or "-all" // arg1: Name of the dump file or null // arg2: parallel thread number -static jint heap_inspection(AttachOperation* op, outputStream* out) { +static void heap_inspection(AttachOperation* op, attachStream* out) { bool live_objects_only = true; // default is true to retain the behavior before this change is made outputStream* os = out; // if path not specified or path is null, use out fileStream* fs = nullptr; @@ -305,8 +465,9 @@ static jint heap_inspection(AttachOperation* op, outputStream* out) { uint parallel_thread_num = MAX2(1, (uint)os::initial_active_processor_count() * 3 / 8); if (arg0 != nullptr && (strlen(arg0) > 0)) { if (strcmp(arg0, "-all") != 0 && strcmp(arg0, "-live") != 0) { + out->set_result(JNI_ERR); out->print_cr("Invalid argument to inspectheap operation: %s", arg0); - return JNI_ERR; + return; } live_objects_only = strcmp(arg0, "-live") == 0; } @@ -325,67 +486,79 @@ static jint heap_inspection(AttachOperation* op, outputStream* out) { if (num_str != nullptr && num_str[0] != '\0') { uint num; if (!Arguments::parse_uint(num_str, &num, 0)) { + out->set_result(JNI_ERR); out->print_cr("Invalid parallel thread number: [%s]", num_str); delete fs; - return JNI_ERR; + return; } parallel_thread_num = num == 0 ? parallel_thread_num : num; } + out->set_result(JNI_OK); + VM_GC_HeapInspection heapop(os, live_objects_only /* request full gc */, parallel_thread_num); VMThread::execute(&heapop); if (os != nullptr && os != out) { out->print_cr("Heap inspection file created: %s", path); delete fs; } - return JNI_OK; } // Implementation of "setflag" command -static jint set_flag(AttachOperation* op, outputStream* out) { +static void set_flag(AttachOperation* op, attachStream* out) { const char* name = nullptr; if ((name = op->arg(0)) == nullptr) { + out->set_result(JNI_ERR); out->print_cr("flag name is missing"); - return JNI_ERR; + return; } FormatBuffer<80> err_msg("%s", ""); int ret = WriteableFlags::set_flag(op->arg(0), op->arg(1), JVMFlagOrigin::ATTACH_ON_DEMAND, err_msg); if (ret != JVMFlag::SUCCESS) { + out->set_result(JNI_ERR); if (ret == JVMFlag::NON_WRITABLE) { out->print_cr("flag '%s' cannot be changed", op->arg(0)); } else { out->print_cr("%s", err_msg.buffer()); } - return JNI_ERR; + return; } - return JNI_OK; + out->set_result(JNI_OK); } // Implementation of "printflag" command // See also: PrintVMFlagsDCmd class -static jint print_flag(AttachOperation* op, outputStream* out) { +static void print_flag(AttachOperation* op, attachStream* out) { const char* name = nullptr; if ((name = op->arg(0)) == nullptr) { + out->set_result(JNI_ERR); out->print_cr("flag name is missing"); - return JNI_ERR; + return; } JVMFlag* f = JVMFlag::find_flag(name); if (f) { + out->set_result(JNI_OK); f->print_as_flag(out); out->cr(); } else { + out->set_result(JNI_ERR); out->print_cr("no such flag '%s'", name); } - return JNI_OK; } // Implementation of "getversion" command -static jint get_version(AttachOperation* op, outputStream* out) { +static void get_version(AttachOperation* op, attachStream* out) { + out->set_result(JNI_OK); out->print("%d", (int)AttachListener::get_supported_version()); - return JNI_OK; + + const char* arg0 = op->arg(0); + if (strcmp(arg0, "options") == 0) { + // print supported options: "option1,option2..." + out->print(" streaming"); + } } // Table to map operation names to functions. @@ -419,10 +592,17 @@ void AttachListenerThread::thread_entry(JavaThread* thread, TRAPS) { assert(thread->stack_base() != nullptr && thread->stack_size() > 0, "Should already be setup"); + AttachListener::set_default_streaming( + get_bool_sys_prop("jdk.attach.vm.streaming", + AttachListener::get_default_streaming(), + thread)); + log_debug(attach)("default streaming output: %d", AttachListener::get_default_streaming() ? 1 : 0); + if (AttachListener::pd_init() != 0) { AttachListener::set_state(AL_NOT_INITIALIZED); return; } + AttachListener::set_initialized(); for (;;) { @@ -433,14 +613,7 @@ void AttachListenerThread::thread_entry(JavaThread* thread, TRAPS) { } ResourceMark rm; - // jcmd output can get lengthy. As long as we miss jcmd continuous streaming output - // and instead just send the output in bulk, make sure large command output does not - // cause asserts. We still retain a max cap, but dimensioned in a way that makes it - // highly unlikely we should ever hit it under normal conditions. - constexpr size_t initial_size = 1 * M; - constexpr size_t max_size = 3 * G; - bufferedStream st(initial_size, max_size); - jint res = JNI_OK; + attachStream st(op->get_reply_writer(), op->streaming_output()); // handle special detachall operation if (strcmp(op->name(), AttachOperation::detachall_operation_name()) == 0) { @@ -458,16 +631,22 @@ void AttachListenerThread::thread_entry(JavaThread* thread, TRAPS) { } if (info != nullptr) { + log_debug(attach)("executing command %s, streaming output: %d", + op->name(), + (op->get_reply_writer() != nullptr && op->streaming_output()) ? 1 : 0); // dispatch to the function that implements this operation - res = (info->func)(op, &st); + info->func(op, &st); + // If the operation handler hasn't set result, set it to JNI_OK now. + // If the result is already set, this is no-op. + st.set_result(JNI_OK); } else { + st.set_result(JNI_ERR); st.print("Operation %s not recognized!", op->name()); - res = JNI_ERR; } + st.complete(); } - // operation complete - send result and output to client - op->complete(res, &st); + op->complete(st.get_result(), st.get_buffered_stream()); } ShouldNotReachHere(); @@ -559,7 +738,7 @@ int AttachOperation::RequestReader::read_uint(bool may_be_empty) { // buffer_size: maximum data size; // min_str_count: minimum number of strings in the request (name + arguments); // min_read_size: minimum data size. -bool AttachOperation::read_request_data(AttachOperation::RequestReader* reader, +bool AttachOperation::RequestReader::read_request_data(AttachOperation* op, int buffer_size, int min_str_count, int min_read_size) { char* buffer = (char*)os::malloc(buffer_size, mtServiceability); int str_count = 0; @@ -568,7 +747,7 @@ bool AttachOperation::read_request_data(AttachOperation::RequestReader* reader, // Read until all (expected) strings or expected bytes have been read, the buffer is full, or EOF. do { - int n = reader->read(buffer + off, left); + int n = read(buffer + off, left); if (n < 0) { os::free(buffer); return false; @@ -600,15 +779,24 @@ bool AttachOperation::read_request_data(AttachOperation::RequestReader* reader, } // Parse request. - // Command name is the 1st string. - set_name(buffer); + // Arguments start at 2nd string. Save it now (option parser can modify 1st argument). + char* arguments = strchr(buffer, '\0') + 1; + // The first string contains command name and (possibly) options. + char* end_of_name = strchr(buffer, ' '); + + if (end_of_name != nullptr) { + parse_options(op, end_of_name + 1); + // zero-terminate command name + *end_of_name = '\0'; + } + op->set_name(buffer); log_debug(attach)("read request: cmd = %s", buffer); // Arguments. char* end = buffer + off; - for (char* cur = strchr(buffer, '\0') + 1; cur < end; cur = strchr(cur, '\0') + 1) { + for (char* cur = arguments; cur < end; cur = strchr(cur, '\0') + 1) { log_debug(attach)("read request: arg = %s", cur); - append_arg(cur); + op->append_arg(cur); } os::free(buffer); @@ -616,8 +804,48 @@ bool AttachOperation::read_request_data(AttachOperation::RequestReader* reader, return true; } -bool AttachOperation::read_request(RequestReader* reader, ReplyWriter* error_writer) { - int ver = reader->read_uint(true); // do not log error if this is "empty" connection +void AttachOperation::RequestReader::parse_options(AttachOperation* op, char* str) { + while (*str != '\0') { + char *name, *value; + str = get_option(str, &name, &value); + log_debug(attach)("option: %s, value: %s", name, value); + + // handle known options + if (strcmp(name, "streaming") == 0) { + if (strcmp(value, "1") == 0) { + op->set_streaming_output(true); + } else if (strcmp(value, "0") == 0) { + op->set_streaming_output(false); + } + } + } +} + +char* AttachOperation::RequestReader::get_option(char* src, char** name, char** value) { + static char empty[] = ""; + // "option1=value1,option2=value2..." + *name = src; + char* end_of_option = strchr(src, ','); + if (end_of_option != nullptr) { + // terminate the option + *end_of_option = '\0'; + // set to next option + src = end_of_option + 1; + } else { + src = empty; + } + char* delim = strchr(*name, '='); + + if (delim != nullptr) { + // terminate option name + *delim = '\0'; + } + *value = delim == nullptr ? empty : (delim + 1); + return src; +} + +bool AttachOperation::RequestReader::read_request(AttachOperation* op, ReplyWriter* error_writer) { + int ver = read_uint(true); // do not log error if this is "empty" connection if (ver < 0) { return false; } @@ -635,12 +863,12 @@ bool AttachOperation::read_request(RequestReader* reader, ReplyWriter* error_wri case ATTACH_API_V2: // 000(0)* (any number of arguments) if (AttachListener::get_supported_version() < 2) { log_error(attach)("Failed to read request: v2 is unsupported or disabled"); - write_reply(error_writer, ATTACH_ERROR_BADVERSION, "v2 is unsupported or disabled"); + error_writer->write_reply(ATTACH_ERROR_BADVERSION, "v2 is unsupported or disabled"); return false; } // read size of the data - buffer_size = reader->read_uint(); + buffer_size = read_uint(); if (buffer_size < 0) { log_error(attach)("Failed to read request: negative request size (%d)", buffer_size); return false; @@ -657,20 +885,20 @@ bool AttachOperation::read_request(RequestReader* reader, ReplyWriter* error_wri break; default: log_error(attach)("Failed to read request: unknown version (%d)", ver); - write_reply(error_writer, ATTACH_ERROR_BADVERSION, "unknown version"); + error_writer->write_reply(ATTACH_ERROR_BADVERSION, "unknown version"); return false; } - bool result = read_request_data(reader, buffer_size, min_str_count, min_read_size); + bool result = read_request_data(op, buffer_size, min_str_count, min_read_size); if (result && ver == ATTACH_API_V1) { // We know the whole request does not exceed buffer_size, // for v1 also name/arguments should not exceed name_length_max/arg_length_max. - if (strlen(name()) > AttachOperation::name_length_max) { + if (strlen(op->name()) > AttachOperation::name_length_max) { log_error(attach)("Failed to read request: operation name is too long"); return false; } - for (int i = 0; i < arg_count(); i++) { - if (strlen(arg(i)) > AttachOperation::arg_length_max) { + for (int i = 0; i < op->arg_count(); i++) { + if (strlen(op->arg(i)) > AttachOperation::arg_length_max) { log_error(attach)("Failed to read request: operation argument is too long"); return false; } @@ -692,23 +920,22 @@ bool AttachOperation::ReplyWriter::write_fully(const void* buffer, int size) { return true; } -bool AttachOperation::write_reply(ReplyWriter * writer, jint result, const char* message, int message_len) { +bool AttachOperation::ReplyWriter::write_reply(jint result, const char* message, int message_len) { if (message_len < 0) { message_len = (int)strlen(message); } char buf[32]; os::snprintf_checked(buf, sizeof(buf), "%d\n", result); - if (!writer->write_fully(buf, (int)strlen(buf))) { + if (!write_fully(buf, (int)strlen(buf))) { return false; } - if (!writer->write_fully(message, message_len)) { + if (!write_fully(message, message_len)) { return false; } - writer->flush(); return true; } -bool AttachOperation::write_reply(ReplyWriter* writer, jint result, bufferedStream* result_stream) { - return write_reply(writer, result, result_stream->base(), (int)result_stream->size()); +bool AttachOperation::ReplyWriter::write_reply(jint result, bufferedStream* result_stream) { + return write_reply(result, result_stream->base(), (int)result_stream->size()); } diff --git a/src/hotspot/share/services/attachListener.hpp b/src/hotspot/share/services/attachListener.hpp index c98491b6ec611..31be87184c14f 100644 --- a/src/hotspot/share/services/attachListener.hpp +++ b/src/hotspot/share/services/attachListener.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -44,16 +44,8 @@ // properties names and values to the output stream). When the function // complets the result value and any result data is returned to the client // tool. - class AttachOperation; -typedef jint (*AttachOperationFunction)(AttachOperation* op, outputStream* out); - -struct AttachOperationFunctionInfo { - const char* name; - AttachOperationFunction func; -}; - enum AttachListenerState { AL_NOT_INITIALIZED, AL_INITIALIZING, @@ -68,6 +60,13 @@ Version 2 (since jdk24): attach operations may have any number of arguments of a To detect if target VM supports version 2, client sends "getversion" command. Old VM reports "Operation not recognized" error, newer VM reports version supported by the implementation. If the target VM does not support version 2, client uses version 1 to enqueue operations. +jdk25 update: client may specify additional options in the attach operation request. + The options are passed as part of the operation command name: "command option1,option2,option3". + "getversion" command with "options" argument returns list of comma-separated options + supported by the target VM. + Option "streaming": + - "streaming=1" turns on streaming output. Output data are sent as they become available. + - "streaming=0" turns off streaming output. Output is buffered and sent after the operation is complete. */ enum AttachAPIVersion: int { ATTACH_API_V1 = 1, @@ -110,10 +109,19 @@ class AttachListener: AllStatic { static AttachAPIVersion _supported_version; + static bool _default_streaming_output; + public: static void set_supported_version(AttachAPIVersion version); static AttachAPIVersion get_supported_version(); + static void set_default_streaming(bool value) { + _default_streaming_output = value; + } + static bool get_default_streaming() { + return _default_streaming_output; + } + static void set_state(AttachListenerState new_state) { Atomic::store(&_state, new_state); } @@ -176,6 +184,7 @@ class AttachOperation: public CHeapObj { private: char* _name; GrowableArrayCHeap _args; + bool _streaming; // streaming output is requested static char* copy_str(const char* value) { return value == nullptr ? nullptr : os::strdup(value, mtServiceability); @@ -198,8 +207,7 @@ class AttachOperation: public CHeapObj { const char* arg(int i) const { // Historically clients expect empty string for absent or null arguments. if (i >= _args.length() || _args.at(i) == nullptr) { - static char empty_str[] = ""; - return empty_str; + return ""; } return _args.at(i); } @@ -214,15 +222,22 @@ class AttachOperation: public CHeapObj { _args.at_put_grow(i, copy_str(arg), nullptr); } + bool streaming_output() const { + return _streaming; + } + void set_streaming_output(bool value) { + _streaming = value; + } + // create an v1 operation of a given name (for compatibility, deprecated) - AttachOperation(const char* name) : _name(nullptr) { + AttachOperation(const char* name) : _name(nullptr), _streaming(AttachListener::get_default_streaming()) { set_name(name); for (int i = 0; i < arg_count_max; i++) { set_arg(i, nullptr); } } - AttachOperation() : _name(nullptr) { + AttachOperation() : _name(nullptr), _streaming(AttachListener::get_default_streaming()) { } virtual ~AttachOperation() { @@ -250,11 +265,22 @@ class AttachOperation: public CHeapObj { // In that case we get "premature EOF" error. // If may_be_empty is true, the error is not logged. int read_uint(bool may_be_empty = false); + + + // Reads standard operation request (v1 or v2), sets properties of the provided AttachOperation. + // Some errors known by clients are reported to error_writer. + bool read_request(AttachOperation* op, ReplyWriter* error_writer); + + private: + bool read_request_data(AttachOperation* op, int buffer_size, int min_str_count, int min_read_size); + // Parses options. + // Note: the buffer is modified to zero-terminate option names and values. + void parse_options(AttachOperation* op, char* str); + // Gets option name and value. + // Returns pointer to the next option. + char* get_option(char* src, char** name, char** value); }; - // Reads standard operation request (v1 or v2). - // Some errors known by clients are reported to error_writer. - bool read_request(RequestReader* reader, ReplyWriter* error_writer); class ReplyWriter { public: @@ -264,14 +290,16 @@ class AttachOperation: public CHeapObj { virtual void flush() {} bool write_fully(const void* buffer, int size); - }; - // Writes standard operation reply (to be called from 'complete' method). - bool write_reply(ReplyWriter* writer, jint result, const char* message, int message_len = -1); - bool write_reply(ReplyWriter* writer, jint result, bufferedStream* result_stream); + // Writes standard operation reply. + bool write_reply(jint result, const char* message, int message_len = -1); + bool write_reply(jint result, bufferedStream* result_stream); + }; -private: - bool read_request_data(AttachOperation::RequestReader* reader, int buffer_size, int min_str_count, int min_read_size); + // Platform implementation needs to implement the method to support streaming output. + virtual ReplyWriter* get_reply_writer() { + return nullptr; + } }; diff --git a/src/hotspot/share/services/diagnosticFramework.cpp b/src/hotspot/share/services/diagnosticFramework.cpp index 47284cd9c0552..a973534c12eae 100644 --- a/src/hotspot/share/services/diagnosticFramework.cpp +++ b/src/hotspot/share/services/diagnosticFramework.cpp @@ -379,15 +379,14 @@ GrowableArray* DCmdParser::argument_info_array() const { DCmdFactory* DCmdFactory::_DCmdFactoryList = nullptr; bool DCmdFactory::_has_pending_jmx_notification = false; -void DCmd::parse_and_execute(DCmdSource source, outputStream* out, - const char* cmdline, char delim, TRAPS) { +void DCmd::Executor::parse_and_execute(const char* cmdline, char delim, TRAPS) { if (cmdline == nullptr) return; // Nothing to do! DCmdIter iter(cmdline, '\n'); int count = 0; while (iter.has_next()) { - if(source == DCmd_Source_MBean && count > 0) { + if(_source == DCmd_Source_MBean && count > 0) { // When diagnostic commands are invoked via JMX, each command line // must contains one and only one command because of the Permission // checks performed by the DiagnosticCommandMBean @@ -408,16 +407,25 @@ void DCmd::parse_and_execute(DCmdSource source, outputStream* out, line = updated_cmd; } - DCmd* command = DCmdFactory::create_local_DCmd(source, line, out, CHECK); + DCmd* command = DCmdFactory::create_local_DCmd(_source, line, _out, CHECK); assert(command != nullptr, "command error must be handled before this line"); DCmdMark mark(command); command->parse(&line, delim, CHECK); - command->execute(source, CHECK); + execute(command, CHECK); } count++; } } +void DCmd::Executor::execute(DCmd* command, TRAPS) { + command->execute(_source, CHECK); +} + +void DCmd::parse_and_execute(DCmdSource source, outputStream* out, + const char* cmdline, char delim, TRAPS) { + Executor(source, out).parse_and_execute(cmdline, delim, CHECK); +} + bool DCmd::reorder_help_cmd(CmdLine line, stringStream &updated_line) { stringStream args; args.print("%s", line.args_addr()); diff --git a/src/hotspot/share/services/diagnosticFramework.hpp b/src/hotspot/share/services/diagnosticFramework.hpp index 16a7ecbe48e10..bd45780e1e54d 100644 --- a/src/hotspot/share/services/diagnosticFramework.hpp +++ b/src/hotspot/share/services/diagnosticFramework.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -278,6 +278,19 @@ class DCmd : public AnyObj { return array; } + // helper class to invoke the framework + class Executor : public StackObj { + DCmdSource _source; + outputStream* _out; + public: + Executor(DCmdSource source, outputStream* out): _source(source), _out(out) {} + + void parse_and_execute(const char* cmdline, char delim, TRAPS); + + protected: + virtual void execute(DCmd* command, TRAPS); + }; + // main method to invoke the framework static void parse_and_execute(DCmdSource source, outputStream* out, const char* cmdline, char delim, TRAPS); diff --git a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java index 52cfd97d7c22b..820af57ec378e 100644 --- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -56,7 +56,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { private static final Path ROOT_TMP = Path.of("root/tmp"); String socket_path; - private int ver = VERSION_1; // updated in ctor depending on detectVersion result + private OperationProperties props = new OperationProperties(VERSION_1); // updated in ctor /** * Attaches to the target VM @@ -124,7 +124,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { checkPermissions(socket_path); if (isAPIv2Enabled()) { - ver = detectVersion(); + props = getDefaultProps(); } else { // Check that we can connect to the process // - this ensures we throw the permission denied error now rather than @@ -178,7 +178,7 @@ InputStream execute(String cmd, Object ... args) throws AgentLoadException, IOEx // connected - write request try { SocketOutputStream writer = new SocketOutputStream(s); - writeCommand(writer, ver, cmd, args); + writeCommand(writer, props, cmd, args); } catch (IOException x) { ioe = x; } diff --git a/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java index e869c08cd91d8..ad6bc14d91dd0 100644 --- a/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java @@ -48,7 +48,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { // Any changes to this needs to be synchronized with HotSpot. private static final String tmpdir; String socket_path; - private int ver = VERSION_1; // updated in ctor depending on detectVersion result + private OperationProperties props = new OperationProperties(VERSION_1); // updated in ctor /** * Attaches to the target VM @@ -109,7 +109,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { checkPermissions(socket_path); if (isAPIv2Enabled()) { - ver = detectVersion(); + props = getDefaultProps(); } else { // Check that we can connect to the process // - this ensures we throw the permission denied error now rather than @@ -161,10 +161,9 @@ InputStream execute(String cmd, Object ... args) throws AgentLoadException, IOEx IOException ioe = null; // connected - write request - // try { SocketOutputStream writer = new SocketOutputStream(s); - writeCommand(writer, ver, cmd, args); + writeCommand(writer, props, cmd, args); } catch (IOException x) { ioe = x; } diff --git a/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java b/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java index ace00100aab99..1afd08f7a3252 100644 --- a/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java +++ b/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -38,6 +38,8 @@ import java.io.IOException; import java.io.InputStreamReader; import java.security.PrivilegedAction; +import java.util.HashMap; +import java.util.Map; import java.util.Properties; import java.util.stream.Collectors; @@ -57,11 +59,17 @@ private static long pid() { } private static final boolean ALLOW_ATTACH_SELF; + private static final boolean ALLOW_STREAMING_OUTPUT; static { String s = VM.getSavedProperty("jdk.attach.allowAttachSelf"); ALLOW_ATTACH_SELF = "".equals(s) || Boolean.parseBoolean(s); + // For now the default is false. + String s2 = VM.getSavedProperty("jdk.attach.allowStreamingOutput"); + ALLOW_STREAMING_OUTPUT = "".equals(s2) || Boolean.parseBoolean(s2); } + private final boolean selfAttach; + HotSpotVirtualMachine(AttachProvider provider, String id) throws AttachNotSupportedException, IOException { @@ -74,9 +82,10 @@ private static long pid() { throw new AttachNotSupportedException("Invalid process identifier: " + id); } + selfAttach = pid == 0 || pid == CURRENT_PID; // The tool should be a different VM to the target. This check will // eventually be enforced by the target VM. - if (!ALLOW_ATTACH_SELF && (pid == 0 || pid == CURRENT_PID)) { + if (!ALLOW_ATTACH_SELF && selfAttach) { throw new IOException("Can not attach to current VM"); } } @@ -331,27 +340,67 @@ public InputStream executeCommand(String cmd, Object ... args) throws IOExceptio protected static final int VERSION_1 = 1; protected static final int VERSION_2 = 2; + // Attach operation properties. + protected static class OperationProperties { + public final static String STREAMING = "streaming"; + + private int ver; + private Map options = new HashMap<>(); + + OperationProperties(int ver) { + this.ver = ver; + } + + int version() { + return ver; + } + + void setOption(String name, String value) { + options.put(name, value); + } + + String options() { + return options.entrySet().stream() + .map(e -> e.getKey() + "=" + e.getValue()) + .collect(Collectors.joining(",")); + } + } + /* - * Detects Attach API version supported by target VM. + * Detects Attach API properties supported by target VM. */ - protected int detectVersion() throws IOException { + protected OperationProperties getDefaultProps() throws IOException { try { - InputStream reply = execute("getversion"); + InputStream reply = execute("getversion", "options"); String message = readMessage(reply); reply.close(); - try { - int supportedVersion = Integer.parseUnsignedInt(message); - // we expect only VERSION_2 - if (supportedVersion == VERSION_2) { - return VERSION_2; + + // Reply is " option1,option2...". + int delimPos = message.indexOf(' '); + String ver = delimPos < 0 ? message : message.substring(0, delimPos); + + int supportedVersion = Integer.parseUnsignedInt(ver); + + // VERSION_2 supports options. + if (supportedVersion == VERSION_2) { + OperationProperties result = new OperationProperties(supportedVersion); + // Parse known options, ignore unknown. + String options = delimPos < 0 ? "" : message.substring(delimPos + 1); + String[] parts = options.split(","); + for (String s: parts) { + if (OperationProperties.STREAMING.equals(s)) { + result.setOption(OperationProperties.STREAMING, + (isStreamingEnabled() ? "1" : "0")); + } } - } catch (NumberFormatException nfe) { - // bad reply - fallback to VERSION_1 + return result; } } catch (AttachOperationFailedException | AgentLoadException ex) { // the command is not supported, the VM supports VERSION_1 only + } catch (NumberFormatException nfe) { + // bad version number - fallback to VERSION_1 } - return VERSION_1; + return new OperationProperties(VERSION_1); } /* @@ -362,6 +411,17 @@ protected boolean isAPIv2Enabled() { return !Boolean.getBoolean("jdk.attach.compat"); } + /* + * Streaming output. + */ + protected boolean isStreamingEnabled() { + // Disable streaming for self-attach. + if (selfAttach) { + return false; + } + return ALLOW_STREAMING_OUTPUT; + } + /* * Utility method to read an 'int' from the input stream. Ideally * we should be using java.util.Scanner here but this implementation @@ -478,9 +538,15 @@ private void writeString(AttachOutputStream writer, Object obj) throws IOExcepti writer.write(b, 0, 1); } - protected void writeCommand(AttachOutputStream writer, int ver, String cmd, Object ... args) throws IOException { - writeString(writer, ver); - if (ver == VERSION_2) { + protected void writeCommand(AttachOutputStream writer, OperationProperties props, + String cmd, Object ... args) throws IOException { + writeString(writer, props.version()); + if (props.version() == VERSION_2) { + // add options to the command name (if specified) + String options = props.options(); + if (!options.isEmpty()) { + cmd += " " + options; + } // for v2 write size of the data int size = dataSize(cmd); for (Object arg: args) { @@ -490,7 +556,7 @@ protected void writeCommand(AttachOutputStream writer, int ver, String cmd, Obje } writeString(writer, cmd); // v1 commands always write 3 arguments - int argNumber = ver == VERSION_1 ? 3 : args.length; + int argNumber = props.version() == VERSION_1 ? 3 : args.length; for (int i = 0; i < argNumber; i++) { writeString(writer, i < args.length ? args[i] : null); } diff --git a/src/jdk.attach/windows/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/windows/classes/sun/tools/attach/VirtualMachineImpl.java index f344be99ccb52..b04d476b0a45d 100644 --- a/src/jdk.attach/windows/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/windows/classes/sun/tools/attach/VirtualMachineImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -43,7 +43,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { private static byte[] stub; private volatile long hProcess; // handle to the process - private int ver = VERSION_1; // updated in ctor depending on detectVersion result + private OperationProperties props = new OperationProperties(VERSION_1); // updated in ctor VirtualMachineImpl(AttachProvider provider, String id) throws AttachNotSupportedException, IOException @@ -55,7 +55,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { try { if (isAPIv2Enabled()) { - ver = detectVersion(); + props = getDefaultProps(); } else { // The target VM might be a pre-6.0 VM so we enqueue a "null" command // which minimally tests that the enqueue function exists in the target @@ -88,12 +88,12 @@ InputStream execute(String cmd, Object ... args) String pipename = pipeprefix + r; long hPipe; try { - hPipe = createPipe(ver, pipename); + hPipe = createPipe(props.version(), pipename); } catch (IOException ce) { // Retry with another random pipe name. r = rnd.nextInt(); pipename = pipeprefix + r; - hPipe = createPipe(ver, pipename); + hPipe = createPipe(props.version(), pipename); } // check if we are detached - in theory it's possible that detach is invoked @@ -105,11 +105,11 @@ InputStream execute(String cmd, Object ... args) try { // enqueue the command to the process. - if (ver == VERSION_1) { - enqueue(hProcess, stub, ver, cmd, pipename, args); + if (props.version() == VERSION_1) { + enqueue(hProcess, stub, props.version(), cmd, pipename, args); } else { // for v2 operations request contains only pipe name. - enqueue(hProcess, stub, ver, null, pipename); + enqueue(hProcess, stub, props.version(), null, pipename); } // wait for the target VM to connect to the pipe. @@ -117,11 +117,11 @@ InputStream execute(String cmd, Object ... args) IOException ioe = null; - if (ver == VERSION_2) { + if (props.version() == VERSION_2) { PipeOutputStream writer = new PipeOutputStream(hPipe); try { - writeCommand(writer, ver, cmd, args); + writeCommand(writer, props, cmd, args); } catch (IOException x) { ioe = x; } diff --git a/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java b/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java new file mode 100644 index 0000000000000..039fea9d61661 --- /dev/null +++ b/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java @@ -0,0 +1,93 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @summary Sanity test for streaming output support + * @bug 8319055 + * @library /test/lib + * @modules jdk.attach/sun.tools.attach + * + * @run main/othervm -Djdk.attach.allowStreamingOutput=true StreamingOutputTest + * @run main/othervm -Djdk.attach.allowStreamingOutput=false StreamingOutputTest + */ + +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.io.IOException; + +import com.sun.tools.attach.VirtualMachine; +import sun.tools.attach.HotSpotVirtualMachine; + +import jdk.test.lib.apps.LingeredApp; + +public class StreamingOutputTest { + + public static void main(String[] args) throws Exception { + boolean clientStreaming = System.getProperty("jdk.attach.allowStreamingOutput").equals("true"); + test(clientStreaming, false); + test(clientStreaming, true); + } + + private static void test(boolean clientStreaming, boolean vmStreaming) throws Exception { + System.out.println("Testing: clientStreaming=" + clientStreaming + ", vmStreaming=" + vmStreaming); + LingeredApp app = null; + try { + app = LingeredApp.startApp("-Xlog:attach=trace", + "-Djdk.attach.vm.streaming=" + String.valueOf(vmStreaming)); + attach(app, clientStreaming, vmStreaming); + } finally { + LingeredApp.stopApp(app); + } + + verify(clientStreaming, vmStreaming, app.getProcessStdout()); + + System.out.println("Testing: end"); + System.out.println(); + } + + private static void attach(LingeredApp app, boolean clientStreaming, boolean vmStreaming) throws Exception { + HotSpotVirtualMachine vm = (HotSpotVirtualMachine)VirtualMachine.attach(String.valueOf(app.getPid())); + try { + try (BufferedReader replyReader = new BufferedReader( + new InputStreamReader(vm.setFlag("HeapDumpPath", "the_path")))) { + System.out.println("vm.setFlag reply:"); + String line; + while ((line = replyReader.readLine()) != null) { + System.out.println("setFlag reply: " + line); + } + } + } finally { + vm.detach(); + } + } + + private static void verify(boolean clientStreaming, boolean vmStreaming, String out) throws Exception { + System.out.println("Target VM output:"); + System.out.println(out); + String expected = "executing command setflag, streaming output: " + (clientStreaming ? "1" : "0"); + if (!out.contains(expected)) { + throw new Exception("VM did not logged expected '" + expected + "'"); + } + } +} diff --git a/test/hotspot/jtreg/serviceability/dcmd/compiler/CodeHeapAnalyticsParams.java b/test/hotspot/jtreg/serviceability/dcmd/compiler/CodeHeapAnalyticsParams.java index 070a3edc7b9ec..5e1addcb298eb 100644 --- a/test/hotspot/jtreg/serviceability/dcmd/compiler/CodeHeapAnalyticsParams.java +++ b/test/hotspot/jtreg/serviceability/dcmd/compiler/CodeHeapAnalyticsParams.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,6 +25,7 @@ /* * @test CodeHeapAnalyticsParams + * @bug 8225388 * @summary Test the Compiler.CodeHeap_Analytics command * @requires vm.flagless * @library /test/lib @@ -38,7 +39,8 @@ public class CodeHeapAnalyticsParams { public static void main(String args[]) throws Exception { PidJcmdExecutor executor = new PidJcmdExecutor(); executor.execute("Compiler.CodeHeap_Analytics all 1").shouldHaveExitValue(0); - executor.execute("Compiler.CodeHeap_Analytics all 0").shouldHaveExitValue(1); - executor.execute("Compiler.CodeHeap_Analytics all k").shouldHaveExitValue(1); + // invalid argument should report exception, and don't crash + executor.execute("Compiler.CodeHeap_Analytics all 0").shouldContain("IllegalArgumentException"); + executor.execute("Compiler.CodeHeap_Analytics all k").shouldContain("IllegalArgumentException"); } } From d9b883a3773d54b7747512b9f434ea98d3666285 Mon Sep 17 00:00:00 2001 From: Alex Menkov Date: Fri, 31 Jan 2025 15:31:32 -0800 Subject: [PATCH 2/3] jcheck --- src/hotspot/share/services/attachListener.cpp | 4 +-- .../tools/attach/HotSpotVirtualMachine.java | 6 ++-- .../AttachAPIv2/StreamingOutputTest.java | 34 +++++++++---------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/hotspot/share/services/attachListener.cpp b/src/hotspot/share/services/attachListener.cpp index 717d54773c9b7..8648411ff66dd 100644 --- a/src/hotspot/share/services/attachListener.cpp +++ b/src/hotspot/share/services/attachListener.cpp @@ -54,7 +54,7 @@ // // To support streaming output platform implementation need to implement AttachOperation::get_reply_writer() method // and ctor allow_streaming argument should be set to true. -// +// // Initially attachStream works in buffered mode. // To switch to the streaming mode attach command handler need to call attachStream::set_result(). // The method flushes buffered output and consequent printing goes directly to ReplyWriter. @@ -357,7 +357,7 @@ static void thread_dump(AttachOperation* op, attachStream* out) { // dispatch to the diagnostic commands used for serviceability functions. static void jcmd(AttachOperation* op, attachStream* out) { JavaThread* THREAD = JavaThread::current(); // For exception macros. - + // All the supplied jcmd arguments are stored as a single // string (op->arg(0)). This is parsed by the Dcmd framework. diff --git a/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java b/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java index 1afd08f7a3252..e0ac0edda513f 100644 --- a/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java +++ b/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java @@ -345,7 +345,7 @@ protected static class OperationProperties { public final static String STREAMING = "streaming"; private int ver; - private Map options = new HashMap<>(); + private Map options = new HashMap<>(); OperationProperties(int ver) { this.ver = ver; @@ -356,11 +356,11 @@ int version() { } void setOption(String name, String value) { - options.put(name, value); + options.put(name, value); } String options() { - return options.entrySet().stream() + return options.entrySet().stream() .map(e -> e.getKey() + "=" + e.getValue()) .collect(Collectors.joining(",")); } diff --git a/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java b/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java index 039fea9d61661..2be2125b93701 100644 --- a/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java +++ b/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java @@ -59,35 +59,35 @@ private static void test(boolean clientStreaming, boolean vmStreaming) throws Ex } finally { LingeredApp.stopApp(app); } - - verify(clientStreaming, vmStreaming, app.getProcessStdout()); - + + verify(clientStreaming, vmStreaming, app.getProcessStdout()); + System.out.println("Testing: end"); System.out.println(); - } - + } + private static void attach(LingeredApp app, boolean clientStreaming, boolean vmStreaming) throws Exception { HotSpotVirtualMachine vm = (HotSpotVirtualMachine)VirtualMachine.attach(String.valueOf(app.getPid())); try { - try (BufferedReader replyReader = new BufferedReader( - new InputStreamReader(vm.setFlag("HeapDumpPath", "the_path")))) { - System.out.println("vm.setFlag reply:"); + try (BufferedReader replyReader = new BufferedReader( + new InputStreamReader(vm.setFlag("HeapDumpPath", "the_path")))) { + System.out.println("vm.setFlag reply:"); String line; while ((line = replyReader.readLine()) != null) { System.out.println("setFlag reply: " + line); } - } + } } finally { vm.detach(); } } - private static void verify(boolean clientStreaming, boolean vmStreaming, String out) throws Exception { - System.out.println("Target VM output:"); - System.out.println(out); - String expected = "executing command setflag, streaming output: " + (clientStreaming ? "1" : "0"); - if (!out.contains(expected)) { - throw new Exception("VM did not logged expected '" + expected + "'"); - } - } + private static void verify(boolean clientStreaming, boolean vmStreaming, String out) throws Exception { + System.out.println("Target VM output:"); + System.out.println(out); + String expected = "executing command setflag, streaming output: " + (clientStreaming ? "1" : "0"); + if (!out.contains(expected)) { + throw new Exception("VM did not logged expected '" + expected + "'"); + } + } } From 052319f91529284c85f55ee71c24e33233bfdc59 Mon Sep 17 00:00:00 2001 From: Alex Menkov Date: Fri, 31 Jan 2025 15:34:52 -0800 Subject: [PATCH 3/3] jcheck --- .../attach/AttachAPIv2/StreamingOutputTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java b/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java index 2be2125b93701..640330823b6a5 100644 --- a/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java +++ b/test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java @@ -59,13 +59,13 @@ private static void test(boolean clientStreaming, boolean vmStreaming) throws Ex } finally { LingeredApp.stopApp(app); } - + verify(clientStreaming, vmStreaming, app.getProcessStdout()); - + System.out.println("Testing: end"); System.out.println(); } - + private static void attach(LingeredApp app, boolean clientStreaming, boolean vmStreaming) throws Exception { HotSpotVirtualMachine vm = (HotSpotVirtualMachine)VirtualMachine.attach(String.valueOf(app.getPid())); try {