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