RFR: 8211752: JNU_ThrowIOExceptionWithLastErrorAndPath - enhance some IOExceptions with path causing the issue
Baesken, Matthias
matthias.baesken at sap.com
Wed Nov 7 08:52:28 UTC 2018
> Sorry, I haven't had time to look at this in more detail yet. But, let's
> take a step back first. Can you or Matthias explain in more detail why
> this fix is necessary? What are the use cases and motivation?
Hello,
adding paths (or maybe more details) to exception messages just makes analyzing errors easier.
You do not get just "Bad path", but also the real bad path which gives you a hint where to look and analyze further .
That's why we introduced it in our JVM ages ago.
I have to agree that additionally printing cwd / user.dir is a bit special, so I omit that from this revision of the patch.
This makes the patch more simple.
New revision :
http://cr.openjdk.java.net/~mbaesken/webrevs/8211752.1/
Unfortunately the usage of sun.security.util.SecurityProperties (which was considered) in the static { ... }
class initializers (e.g. UnixFileSystem.java) just does not work.
It fails with already in the build (!) with :
Error occurred during initialization of boot layer
java.lang.ExceptionInInitializerError
Caused by: java.lang.NullPointerException
(seems it is too early in the game for SecurityProperties).
(btw. this is another NOT very helpful exception error message)
So I unfortunately had to go back to using system properties.
Btw. another question regarding path output in exceptions :
you seem to consider it a bad thing to (unconditionally) print paths in the exception messages,
but then on the other hand we have it already in OpenJDK UnixFileSystem_md.c :
269 JNIEXPORT jboolean JNICALL
270 Java_java_io_UnixFileSystem_createFileExclusively(JNIEnv *env, jclass cls,
271 jstring pathname)
272 {
.......
277 /* The root directory always exists */
278 if (strcmp (path, "/")) {
279 fd = handleOpen(path, O_RDWR | O_CREAT | O_EXCL, 0666);
280 if (fd < 0) {
281 if (errno != EEXIST)
282 JNU_ThrowIOExceptionWithLastError(env, path);
283 } else {
284 if (close(fd) == -1)
285 JNU_ThrowIOExceptionWithLastError(env, path);
Why is it fine here for a long time , but considered harmful at the other places ?
If we want to be consistent, we should then write "Bad path" here (or allow the path output at the other places too ).
Thanks, Matthias
> -----Original Message-----
> From: Sean Mullan <sean.mullan at oracle.com>
> Sent: Freitag, 12. Oktober 2018 17:19
> To: Langer, Christoph <christoph.langer at sap.com>; Baesken, Matthias
> <matthias.baesken at sap.com>; Alan Bateman <Alan.Bateman at oracle.com>;
> security-dev at openjdk.java.net; core-libs-dev at openjdk.java.net
> Subject: Re: RFR: 8211752: JNU_ThrowIOExceptionWithLastErrorAndPath -
> enhance some IOExceptions with path causing the issue
>
> On 10/12/18 10:33 AM, Langer, Christoph wrote:
> > Sean, what is your take on this?
>
> Sorry, I haven't had time to look at this in more detail yet. But, let's
> take a step back first. Can you or Matthias explain in more detail why
> this fix is necessary? What are the use cases and motivation? The bug
> report doesn't go into any detail about that and there isn't anything
> in the initial RFR email that explains why this change is useful or
> necessary. As a general guideline or advice, RFEs should include this
> type of information so that Reviewers understand more of the context and
> motivation behind the change.
>
> Thanks,
> Sean
More information about the core-libs-dev
mailing list