RFR: 8259070: Add jcmd option to dump CDS [v12]

Yumin Qi minqi at openjdk.java.net
Thu Apr 15 05:24:38 UTC 2021


On Sat, 27 Feb 2021 18:20:52 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Hi Yumin,
>> 
>> This is a very useful addition.
>> 
>> My biggest concern with this patch though is the use of `os::fork_and_exec()` in regular, non-fatal situations. I had a look at that function and I think that is very unsafe.
>> 
>> - it does not close file descriptors, so the child vm will inherit all file descriptors not opened with FD_CLOEXEC. In Runtime.exec, we do this whole dance around safely closing off all unused file descriptors, which is missing here. Note that even though we mostly open all fds with CLOEXEC, this does not matter, since user code may not do that.
>> - It uses vfork "if available", which is probably always, but that may be okay since the child exec's right away. Still, vfork makes me nervous.
>> - Weirdly enough, it always spawns the child program via one indirection using a shell; so there is always the shell between you and your spawned VM, and you probably wont get the return code of your child vm process back.
>>  
>> Until now, os::fork_and_exec() was only used in fatal situations where the VM was about to die anyway. And where it did not really matter if it worked or not. 
>> 
>> If we now want to use it in regular situations, we need to give that thing an overhaul and make it a first class fork api, and also test it better that it is today. The file descriptor issue has to be addressed at the very least.
>> 
>> I really would consider rewriting the whole thing using posix_spawn. Since JDK15 I think posix_spawn is the default for Runtime.exec, so we know it works well. I would also do this in a separate RFE.
>> 
>> Alternatively, you could call into java and use Runtime.exec().
>> 
>> Further remarks inline.
>> 
>> Thanks, Thomas
>
>> I really would consider rewriting the whole thing using posix_spawn. Since JDK15 I think posix_spawn is the default for Runtime.exec, so we know it works well. I would also do this in a separate RFE.
>> 
>> Alternatively, you could call into java and use Runtime.exec().
> 
> I think we should call into Java and use `Runtime.exec()`. Running a subprocess is very complicated, so we shouldn't try to duplicate the code in the VM.

@iklam @calvinccheung @Hamlin-Li Thanks for review!

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

PR: https://git.openjdk.java.net/jdk/pull/2737


More information about the serviceability-dev mailing list