fopen vs. os::fopen and automatic closing of the file on exec

[David Holmes]

Hi Matthias,

I don't have any info of most below but one follow up ....

On 28/01/2020 10:51 pm, Baesken, Matthias wrote:
> Hello, I noticed   while looking at   https://bugs.openjdk.java.net/browse/JDK-8237830   ( support O_CLOEXEC in os::open on other OS than Linux )
> that os::fopen  also  has  some support  for setting  FD_CLOEXEC / O_CLOEXEC  on the file opened .
> See :
> 
> 1253// This function is a proxy to fopen, it tries to add a non standard flag ('e' or 'N')
> 1254// that ensures automatic closing of the file on exec. If it can not find support in
> 1255// the underlying c library, it will make an extra system call (fcntl) to ensure automatic
> 1256// closing of the file on exec.
> 1257FILE* os::fopen(const char* path, const char* mode) {
> 1258  char modified_mode[20];
> 1259  assert(strlen(mode) + 1 < sizeof(modified_mode), "mode chars plus one extra must fit in buffer");
> 1260  sprintf(modified_mode, "%s" LINUX_ONLY("e") BSD_ONLY("e") WINDOWS_ONLY("N"), mode);
> 1261  FILE* file = ::fopen(path, modified_mode);
> 1262
> 1263#if !(defined LINUX || defined BSD || defined _WINDOWS)
> 1264  // assume fcntl FD_CLOEXEC support as a backup solution when 'e' or 'N'
> 1265  // is not supported as mode in fopen
> 1266  if (file != NULL) {
> 1267    int fd = fileno(file);
> 1268    if (fd != -1) {
> 1269      int fd_flags = fcntl(fd, F_GETFD);
> 1270      if (fd_flags != -1) {
> 1271        fcntl(fd, F_SETFD, fd_flags | FD_CLOEXEC);
> 1272      }
> 1273    }
> 1274  }
> 1275#endif
> 
> However some questions arise here :
> 
>    1.  Usage :        os::fopen   is  only used sometimes  in HS code ,  should most of the calls to  fopen be adjusted to os::fopen   (see list  below )
>    2.  ::fopen vs. ::fcntl     :    is  os_linux  os::open   we try to  set  the  "closing of the file on exec"   flag  when calling ::open  but we later check  that it really  worked so we seem not to trust it fully ;

The check is for running on older Linuxes that do not support O_CLOEXEC 
- where the flag is ignored. That is why I asked about what happens on 
BSD/macOS and AIX in that situation.

> Should this be done here too  for Linux ? Or is that  checking in os_linux os::open  these days not needed any more ?

It's possible the most recent version of Linux without O_CLOEXEC 
supported is no longer supported by OpenJDK, in which case we can remove 
it. But I'm not sure what that version is. I have no idea if fopen with 
"e" support has the same history as ::open and O_CLOEXEC.

David

> Best regards, Matthias
> 
> 
> 
> Grep showed  me these fopen-calls in HS, a lot go not to os::fopen ?
> 
> cpu/aarch64/vm_version_aarch64.cpp:171:  if (FILE *f = fopen("/proc/cpuinfo", "r")) {
> cpu/ppc/vm_version_ppc.cpp:412:  FILE* fp = fopen(info_file, "r");
> os/aix/os_aix.cpp:3756:  // - might cause an fopen in the subprocess to fail on a system
> os/aix/os_perf_aix.cpp:243:  if ((f = fopen(procfile, "r")) == NULL) {
> os/aix/os_perf_aix.cpp:666:  if ((fp = fopen(buffer, "r")) != NULL) {
> os/aix/os_perf_aix.cpp:694:  if ((fp = fopen(buffer, "r")) != NULL) {
> os/bsd/os_bsd.cpp:3479:  // - might cause an fopen in the subprocess to fail on a system
> os/linux/decoder_linux.cpp:61:  FILE* file = fopen(filepath, "r");
> os/linux/os_linux.cpp:257:  if ((fh = fopen("/proc/stat", "r")) == NULL) {
> os/linux/os_linux.cpp:364:    FILE *fp = fopen(fname, "r");
> os/linux/os_linux.cpp:1106:  FILE *fp = fopen("/proc/self/maps", "r");
> os/linux/os_linux.cpp:1218:    fp = fopen("/proc/self/stat", "r");
> os/linux/os_linux.cpp:2075:  if ((procmapsFile = fopen("/proc/self/maps", "r")) != NULL) {
> os/linux/os_linux.cpp:2244:  FILE* fp = fopen(file, "r");
> os/linux/os_linux.cpp:2458:  FILE *fp = fopen("/proc/cpuinfo", "r");
> os/linux/os_linux.cpp:2515:  FILE* fp = fopen("/proc/cpuinfo", "r");
> os/linux/os_linux.cpp:3645:    FILE *fp = fopen("/proc/self/maps", "r");
> os/linux/os_linux.cpp:3689:  if ((f = fopen("/proc/self/coredump_filter", "r+")) == NULL) {
> os/linux/os_linux.cpp:3741:  FILE *fp = fopen("/proc/meminfo", "r");
> os/linux/os_linux.cpp:5572:  // - might cause an fopen in the subprocess to fail on a system
> os/linux/os_linux.cpp:5797:  fp = fopen(proc_name, "r");
> os/linux/os_perf_linux.cpp:238:  if ((f = fopen(procfile, "r")) == NULL) {
> os/linux/os_perf_linux.cpp:275:  if ((f = fopen("/proc/stat", "r")) == NULL) {
> os/linux/os_perf_linux.cpp:726:  if ((fp = fopen(buffer, "r")) != NULL) {
> os/linux/os_perf_linux.cpp:754:  if ((fp = fopen(buffer, "r")) != NULL) {
> os/linux/perfMemory_linux.cpp:659:  FILE *fp = fopen(fname, "r");
> os/linux/gc/z/zPhysicalMemoryBacking_linux.cpp:312:  FILE* const file = fopen(filename, "r");
> os/linux/gc/z/zMountPoint_linux.cpp:72:  FILE* fd = fopen(PROC_SELF_MOUNTINFO, "r");
> os/linux/cgroupSubsystem_linux.cpp:66:  cgroups = fopen("/proc/cgroups", "r");
> os/linux/cgroupSubsystem_linux.cpp:121:  cgroup = fopen("/proc/self/cgroup", "r");
> os/linux/cgroupSubsystem_linux.cpp:170:    mntinfo = fopen("/proc/self/mountinfo", "r");
> os/linux/cgroupSubsystem_linux.cpp:224:  mntinfo = fopen("/proc/self/mountinfo", "r");
> os/linux/cgroupSubsystem_linux.hpp:96:  fp = fopen(file, "r");
> os/solaris/os_perf_solaris.cpp:521:      if ((fp = fopen(buffer, "r")) != NULL) {
> os/solaris/os_perf_solaris.cpp:557:  if ((fp = fopen(psinfo_path, "r")) == NULL) {
> os/solaris/os_solaris.cpp:1605:  FILE* fp = fopen("/etc/release", "r");
> os/solaris/os_solaris.cpp:4091:  //   fopen must be less than 256, _even_ when the first limit above
> os/solaris/os_solaris.cpp:4092:  //   has been raised.  This can cause calls to fopen (but not calls to
> os/solaris/os_solaris.cpp:4094:  //   native code (although the JDK itself uses fopen).  One can hardly
> os/solaris/os_solaris.cpp:4103:  //   stdio fopen limit by calling function enable_extended_FILE_stdio.
> os/solaris/os_solaris.cpp:4138:  // - might cause an fopen in the subprocess to fail on a system
> os_cpu/linux_sparc/vm_version_linux_sparc.cpp:39:    FILE* fp = fopen("/proc/cpuinfo", "r");
> share/logging/logFileOutput.cpp:272:  _stream = os::fopen(_file_name, FileOpenMode);
> share/logging/logFileOutput.cpp:361:  _stream = os::fopen(_file_name, FileOpenMode);
> share/runtime/arguments.cpp:1345:  FILE* stream = fopen(file_name, "rb");
> share/runtime/memprofiler.cpp:76:    _log_fp = fopen(log_name , "w+");
> share/runtime/os.cpp:1253:// This function is a proxy to fopen, it tries to add a non standard flag ('e' or 'N')
> share/runtime/os.cpp:1257:FILE* os::fopen(const char* path, const char* mode) {
> share/runtime/os.cpp:1261:  FILE* file = ::fopen(path, modified_mode);
> share/runtime/os.cpp:1265:  // is not supported as mode in fopen
> share/runtime/os.hpp:510:  static FILE* fopen(const char* path, const char* mode);
> share/runtime/abstract_vm_version.cpp:308:  FILE* fp = fopen(filename, "r");
> share/utilities/elfFile.cpp:171:  _file = fopen(filepath, "r");
> share/utilities/ostream.cpp:513:  _file = fopen(file_name, "w");
> share/utilities/ostream.cpp:523:  _file = fopen(file_name, opentype);
> share/adlc/main.cpp:363:      (ADF._fp = fopen(ADF._name, action)) == NULL) {
> share/c1/c1_Compilation.cpp:697:  fileStream stream(fopen("c1_compile_only", "wt"));
> share/c1/c1_Compilation.cpp:713:  fileStream stream(fopen(".hotspot_compiler", "at"));
> share/ci/ciReplay.cpp:127:    _stream = fopen(filename, "rt");
> share/classfile/classListParser.cpp:51:  // Use os::open() because neither fopen() nor os::fopen()
> share/compiler/compileBroker.cpp:1891:      fp = fopen(file_name, "wt");
> share/compiler/compilerOracle.cpp:703:  FILE* stream = fopen(cc_file(), "rt");
> share/compiler/disassembler.cpp:272:        if ((fp = fopen(file, "r")) == NULL) {
>