RFR: 8324683: Unify AttachListener code for Posix platforms [v4]

[Magnus Ihse Bursie]

On Mon, 15 Apr 2024 13:19:17 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:

>> Hi folks,
>> 
>> This PR aims to unify a lot of the shared code between ```attachListener_linux.cpp``` and ```attachListener_bsd.cpp```. 
>> 
>> Some key points:
>> 
>> 1. AIX: 
>> 
>> ```attachListener_aix.cpp``` has some key differences from linux/bsd, so I have reduced the scope of the issue to linux and bsd only. As a result, there's an ifndef AIX statement wrapping the whole file. Unifying AIX could be a separate issue.
>> 
>> 2. Peer credentials: 
>> 
>> In ```AttachListener::dequeue```, bsd and linux use different APIs to obtain peer credentials. Linux uses ```getsockopt```, while bsd uses ```getpeerid```. 
>> 
>> Bsd doesn't have [SO_PEERCRED ](https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getsockopt.2.html). While Linux has ```getpeerid``` implemented on top of ```getsockopt```, it comes in the [bsd compat lib](https://cgit.freedesktop.org/libbsd/tree/src/getpeereid.c). 
>> 
>> To avoid adding further dependencies, I have kept both API calls separated by an ifdef macro. 
>> 
>> Additionally, there are some slight differences in the logging messages in this area for bsd and linux. It's possible to coalesce them should this make more sense. 
>> 
>> 3. ```AttachListener::is_init_trigger()```
>> 
>> Please refer to [mailing list discussion](https://mail.openjdk.org/pipermail/hotspot-runtime-dev/2024-March/068413.html) for details. 
>> 
>> Per the discussion, I tried removing the check for ".attach_pid" in the current working directory (more closely resembling what the bsd code does). However, I noted this caused many failure across linux hotspot tests. See GHA job with failures [here](https://github.com/SoniaZaldana/jdk/actions/runs/8349574000). 
>> 
>> Therefore, I opted for the Linux approach in this PR. This means we do the check in the current working directory across bsd and linux, although this has been removed from bsd before. This seemingly doesn't break any bsd tests.
>> 
>> 5. Minimal JVM Variant: 
>> 
>> I came across an error in the minimal JVM build:
>> ` error: invalid use of incomplete type ‘class AttachOperation’
>>    80 | class PosixAttachOperation : public AttachOperation {`
>> 
>> After a bit of digging, I found that we only get complete definition of AttachOperation inside the directive ```#IF INCLUDE_SERVICES``` [here](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/services/attachListener.hpp#L138 ). 
>> 
>> I added the same directive for the ```attachListener_posix.cpp``` file but I’m not entirel...
>
> Sonia Zaldana Calles has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Changing name from posix to nix and updating endif comment

This went under my radar (even though I try to keep a watch out for platform cleanups like this). Just let me add a few words on the "posix/xnix/nix/unix" discussion.

In the rest of the JDK (outside hotspot), the decision was made to use the name "unix" for stuff that are shared between, well, unixes (linux, macos, bsd, aix, solaris). 

The reason people have historically been avoiding "unix" and using things like "xnix" etc was due to fear of legal implications. Mark apparently managed to get clearance from Oracle's legal department to use "unix" in our filenames, so with that worry out of the way, "unix" was the obvious choice for what everyone thinks of as "unix" anyway. This change was done in the big reshuffling of source code preceding Project Jigsaw (modules support).

Hotspot mostly stayed out of that reshuffling, and so the name "posix" was still kept in Hotspot. In the build system, we basically have a piece of code saying "if we are compiling hotspot, then replace `unix` with `posix`". Personally, I think hotspot should go the same route as the rest of the JDK and replace "posix" with "unix". But maybe that will never happen, and there are also a lot of other differences in platform naming, so fixing just one is perhaps not relevant.

In any case, I strongly recommend that you  keep to the "hotspot tradition" and use "posix" as a general name for stuff that are shared across unixes, in the file/class name just as in the path name. Yes, not everything there is defined by the POSIX standard, but that is most likely the case for more shared "posix" code already (that is why "unix" is arguably better).

-------------

PR Comment: https://git.openjdk.org/jdk/pull/18283#issuecomment-2063523915