RFR: 8280881: (fs) UnixNativeDispatcher.close0 may throw UnixException [v2]
Brian Burkhalter
bpb at openjdk.java.net
Wed Feb 16 00:56:12 UTC 2022
On Tue, 15 Feb 2022 18:02:26 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:
>> src/java.base/unix/classes/sun/nio/fs/UnixUserDefinedFileAttributeView.java line 139:
>>
>>> 137: close(fd);
>>> 138: } catch (UnixException y) {
>>> 139: y.rethrowAsIOException(file);
>>
>> Hello Brian,
>> A few lines above this code in the finally block, there's a catch block which catches the `UnixException` and rethrows it as a `FileSystemException`. With the change being done in this finally block, this finally block now potentially can throw an `IOException` from a failed call to `close()` and thus the original `FileSystemException` can get lost. Do you think it would be better to keep track any potential `FileSystemException` from the previous catch block and add this `IOException` from this `close()` call as a suppressed exception to that `FileSystemException` and rethrow that original exception? Loosely, something like:
>>
>>
>> IOException failure = null;
>> try {
>> ....
>>
>> } catch (UnixException x) {
>> failure = new FileSystemException(file.getPathForExceptionMessage(),
>> null, "Unable to get list of extended attributes: " +
>> x.getMessage());
>> } finally {
>> try {
>> close(fd);
>> } catch (UnixException y) {
>> if (failure == null) {
>> // throw the exception that happened during close
>> y.rethrowAsIOException(file);
>> } else {
>> // add the failure to close as a suppressed exception to
>> // the previously caught failure
>> failure.addSuppressed(y.asIOException(file));
>> }
>> }
>> // throw any previously caught failure
>> if (failure != null) {
>> throw failure;
>> }
>> }
>>
>>
>> This applies to few others places that have changed in this PR.
>
> This might be a good idea. I think I need to revisit all the changes in this patch.
In commit 0e7b72f I elected to do the following:
1. If `close()` throws a `UnixException` in a finally block, then it is ignored unless it can be ascertained that it would would _not_ mask an exception thrown in the try block, in which case it is rethrown as an `IOException`.
2. The `UnixException` is also ignored if the calling method does not throw `IOException`.
3. If the `UnixException` can be added as a suppressed exception to a preexisting `IOException` then that is done.
4. Otherwise if the `UnixException` can be rethrown as an `IOException` without masking another exception than that is done.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7467
More information about the nio-dev
mailing list