RFR: 8341282: (fs) Move creation time fallback logic to Java layer (Linux) [v3]

Severin Gehwolf sgehwolf at openjdk.org
Wed Oct 2 09:12:35 UTC 2024


On Tue, 1 Oct 2024 19:57:52 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Move the decision as to whether `BasicFileAttributes.creationTime()` falls back to the modified time from the native layer to the Java layer.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8341282: Unset birthtime_available when using stat(2) on Linux

If I'm understanding this correctly the default value of `birthtime_available = true` is to account for the fact that on MacOSX birth time support is not conditional on the filesystem. It's assumed to always be supported. On Linux, on the other hand, it's filesystem dependent.

What's strange is that in https://github.com/openjdk/jdk/pull/21274/commits/22ac46eab211ca32ab046283c6a7efcf215425a8 we set birth time support to false, but we should never enter this code `copy_stat_attributes()` on Linux since we set the capability i.e. `UnixNativeDispatcher.birthtimeSupported()` to true iff `statx` is available and then we'd use `copy_statx_attributes()` instead. So it makes little sense to set both on Linux: capabilities and `birthtime_available` in `copy_stat_attributes()` since it should never end up there for the birth time supported case.

How about keeping `birthtime_available` default `false` (no explicit initialization) and do this in `copy_stat_attributes()`:


static void copy_stat_attributes(JNIEnv* env, struct stat* buf, jobject attrs) {
    (*env)->SetIntField(env, attrs, attrs_st_mode, (jint)buf->st_mode);
    (*env)->SetLongField(env, attrs, attrs_st_ino, (jlong)buf->st_ino);
    (*env)->SetLongField(env, attrs, attrs_st_dev, (jlong)buf->st_dev);
    (*env)->SetLongField(env, attrs, attrs_st_rdev, (jlong)buf->st_rdev);
    (*env)->SetIntField(env, attrs, attrs_st_nlink, (jint)buf->st_nlink);
    (*env)->SetIntField(env, attrs, attrs_st_uid, (jint)buf->st_uid);
    (*env)->SetIntField(env, attrs, attrs_st_gid, (jint)buf->st_gid);
    (*env)->SetLongField(env, attrs, attrs_st_size, (jlong)buf->st_size);
    (*env)->SetLongField(env, attrs, attrs_st_atime_sec, (jlong)buf->st_atime);
    (*env)->SetLongField(env, attrs, attrs_st_mtime_sec, (jlong)buf->st_mtime);
    (*env)->SetLongField(env, attrs, attrs_st_ctime_sec, (jlong)buf->st_ctime);
#ifdef _DARWIN_FEATURE_64_BIT_INODE
   // birthtime_available defaults to 'false'. On Darwin, it's unconditionally true
   (*env)->SetBooleanField(env, attrs, attrs_birthtime_available,
                            (jboolean)JNI_TRUE);
    (*env)->SetLongField(env, attrs, attrs_st_birthtime_sec, (jlong)buf->st_birthtime);
    // rely on default value of 0 for st_birthtime_nsec field on Darwin
#endif


This would avoid the confusing part of having to think about why the Linux code ends up dealing with birth time support in `copy_stat_attributes()` to begin with.

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

PR Review: https://git.openjdk.org/jdk/pull/21274#pullrequestreview-2342274026


More information about the nio-dev mailing list