On Tue, 5 Nov 2024 00:42:23 GMT, Alex Menkov <amen...@openjdk.org> wrote:
> The fix adds correct handling of "could not open pipe" errors in attach
> listener instead of assert which causes target VM crash.
>
> testing: tier1,tier2,hs-tier5-svc
Hi,
List of tests doesn't mention tier4 where the failures were first noticed?
When PipeChannel::open logs error, and returns false, then
Win32AttachOperation::open_pipe returns that value, then
Win32AttachListener::dequeue() also logs that there was an error?
Is that message just a less detailed duplicate?
(it states whether it's v1 or v2, but maybe that's not that interesting?)
In the v2 case, Win32AttachListener::dequeue() will log if open succeeded, if
!op->read_request().
But AttachOperation::read_request mostly logs its own errors as well.
I think it's only this one:
639 if (buffer_size < 0) {
640 return false;
...where it does not log when returning false. Maybe it should log then, and
Win32AttachListener::dequeue() does not need to log?
While are are here, is it possible to address these at the same time?
open/src/hotspot/share/services/attachListener.cpp
typo "ot" -> "or"
631 case ATTACH_API_V2: // <ver>0<size>0<cmd>0<arg>0<arg>0<arg>0
632 if (AttachListener::get_supported_version() < 2) {
633 log_error(attach)("Failed to read request: v2 is unsupported ot
disabled");
exact -> "exactly"
649 // Must contain exact 'buffer_size' bytes.
Also, is the line 631 comment, with 3 args, misleading? (moving away from
exactly 3 arguments...)
Thanks!
-------------
PR Review: https://git.openjdk.org/jdk/pull/21888#pullrequestreview-2415229062
