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

[David Holmes]

On 29/01/2020 7:38 pm, Baesken, Matthias wrote:
> Hi David , yes you are probably right, simply adding it always in the jdk  coding  would lead to problems .
> On the other hand,  do I  have the  option  to set O_CLOEXEC in some way for  the jdk  open calls , in case I want it ?

Not O_CLOEXEC AFAICS:

https://docs.oracle.com/en/java/javase/13/docs/api/java.base/java/nio/file/StandardOpenOption.html

David

> Best regards, Matthias
> 
> 
> 
>> Hi Matthias,
>>
>> On 28/01/2020 11:44 pm, Baesken, Matthias wrote:
>>> Hi David, thanks for looking into it .
>>>
>>> While checking other  open(64) calls, I found  the fileOpen / handleOpen
>> calls  in java.base .
>>> Looks like  they  miss  so far  setting  FD_CLOEXEC /   O_CLOEXEC  , it isn’t
>> done in the callers of those functions  and it is also not (silently) added in the
>> fileOpen/handleOpen function itself  (like HS does in os::open ).
>>> See :
>>>
>>> 73FD
>>> 74handleOpen(const char *path, int oflag, int mode) {
>>> 75    FD fd;
>>> 76    RESTARTABLE(open64(path, oflag, mode), fd);
>>> 77    if (fd != -1) {
>>> 78        struct stat64 buf64;
>>> 79        int result;
>>> 80        RESTARTABLE(fstat64(fd, &buf64), result);
>>> 81        if (result != -1) {
>>> 82            if (S_ISDIR(buf64.st_mode)) {
>>> 83                close(fd);
>>> 84                errno = EISDIR;
>>> 85                fd = -1;
>>> 86            }
>>> 87        } else {
>>> 88            close(fd);
>>> 89            fd = -1;
>>> 90        }
>>> 91    }
>>> 92    return fd;
>>> 93}
>>>
>>> 95void
>>> 96fileOpen(JNIEnv *env, jobject this, jstring path, jfieldID fid, int flags)
>>> 97{
>>> .....
>>> 107        fd = handleOpen(ps, flags, 0666);
>>> ....
>>> 122}
>>>
>>> 56JNIEXPORT void JNICALL
>>> 57Java_java_io_FileOutputStream_open0(JNIEnv *env, jobject this,
>>> 58                                    jstring path, jboolean append) {
>>> 59    fileOpen(env, this, path, fos_fd,
>>> 60             O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC));
>>> 61}
>>>
>>> 59JNIEXPORT void JNICALL
>>> 60Java_java_io_FileInputStream_open0(JNIEnv *env, jobject this, jstring
>> path) {
>>> 61    fileOpen(env, this, path, fis_fd, O_RDONLY);
>>> 62}
>>>
>>>
>>> Is this something that should be changed too ?
>>
>> I think not. Unless those public API's are specified to open a file in a
>> specific mode, like close-on-exec, then they should not do that by default.
>>
>> We should only be doing close-on-exec on internally used files.
>>
>> David
>> -----
>