Don't leak temporary file during test and inital loading of lib. #141
Conversation
Motivation: We did leak a file during our tests and also during the initial loading of the native lib Modifications: Use a tempory file / directory and so ensure the file is deleted Result: No leaked file
transport-native-io_uring/src/test/java/io/netty/incubator/channel/uring/NativeTest.java
Outdated
Show resolved
Hide resolved
…nnel/uring/NativeTest.java
| try { | ||
| // First, try calling a side-effect free JNI method to see if the library was already | ||
| // loaded by the application. | ||
| Native.createFile(); | ||
| Native.createFile(tmpFile.toString()); |
There was a problem hiding this comment.
The comment says "side-effect free" but creating a file is not. How did we get here?
There was a problem hiding this comment.
Also, this returns a file descriptor that isn't closed.
There was a problem hiding this comment.
side-effect free in terms of JNI.
| static jint netty_create_file(JNIEnv *env, jclass class, jstring filename) { | ||
| const char *file = (*env)->GetStringUTFChars(env, filename, 0); | ||
|
|
||
| int fd = open(file, O_RDWR | O_TRUNC | O_CREAT, 0644); |
There was a problem hiding this comment.
Since we're Linux specific (because io_uring) we can use O_TMPFILE and have the "file" refer to the parent directory. That way the file will never have a name, and will vanish once the fd is closed.
There was a problem hiding this comment.
good point ... let me do this
There was a problem hiding this comment.
It is a good idea but seems requires some checks to be sure it works
_TMPFILE requires support by the underlying filesystem;
only a subset of Linux filesystems provide that support.
In the initial implementation, support was provided in the
ext2, ext3, ext4, UDF, Minix, and tmpfs filesystems.
Support for other filesystems has subsequently been added
as follows: XFS (Linux 3.15); Btrfs (Linux 3.16); F2FS
(Linux 3.16); and ubifs (Linux 4.9)
but it seems vastly supported TBH
There was a problem hiding this comment.
@franz1981 thanks a lot! Because of this I just did not change anything but added a explicit delete to the test.
Motivation:
We did leak a file during our tests and also during the initial loading of the native lib
Modifications:
Use a tempory file / directory and so ensure the file is deleted
Result:
No leaked file