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

[Baesken, Matthias]

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 ;
Should this be done here too  for Linux ? Or is that  checking in os_linux os::open  these days not needed any more ?

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) {