On Fri, 18 Dec 2020 21:30:28 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> The behaviour may not be identical but it is non-identical in theoretical >> fatal cases. >> We are actually better off here. I am seeing fewer JNI warning as a result. >> Also NS exceptions per Apple should result in termination of the process. >> I don't think JNF ever did the right thing. >> So as I am sure you know this is only ever going to come into play if there >> is some JNI >> error which indicates an internal error. I don't think we are any worse off >> here. > > I see a mixing approach here, on other platforms like on windows we: > - Use such macro in the initid() method in the static initializers and any > errors are fatal in that case. And we usually use the macro instead of > functions, because it is possible to check and return from the initid(). > - If some exception like OOM has occurred on the toolkit thread we wrap it > to the std::bad_alloc end stop the application. > > The JNF solution on macOS merges two solutions above, so we call JNI methods > in any place and the library transforms all possible java exceptions to the > NSExceptions, and back. And it was quite useful, much better than the list of > macro. I did analyse what seems to be happening with the existing JNF code and am comfortable with it. The use of NSExceptions in that case is really against the grain of what Apple say for NSExceptions that they are meant for terminal conditions, so it is a bit weird to just use them to "carry" a Java Exception. However if we do see one on return (the EXIT code yet to be updated) then we would still turn it into a Java exception. I am against converting a Java Exception into an NSException then back into a Java exception which seems unnecessary. >> If we get NULL back then there's a progamming bug. LOG_NULL will show it. >> CHECK_NULL is just as much to satisfy JNI as anything. >> If we are on a Java thread then it will still add some value so I don't see >> the point in removing it. >> Nor do I think I'd like to use NSException instead because that affects the >> flow of control. >> Yes, this may be different than before (probably more relevant to the other >> comment) but I think better may be a better approach overall. If we ever see >> an issue here that is likely to occur due to anything other than a bad >> internal inconsistency due to JDK changes then I would prefer to follow the >> code path and fix it up. > > I mean all macro, not just CHECK_NULL. You just create a JNF library 1.5 > which for some reason uses macro instead of objective-c functions, uses > logging instead of exceptions, and changes a huge number of files. Does it > make the codebase better? > - I think we should ask Apple to contribute the JNF library or merge it as a > third-party library, then cut all unused parts of it > - Or we should create JNF library 2.0, and the whole fix will be looking > like a replacement of three letters in each function, like JNFCallVoidMethod > - > JNUCallVoidMethod etc. I have to disagree. (1) is a non-starter and (2) is un-needed and I have no desire to copy the patterns in JNF which I don't think are needed. This will end up simpler. ------------- PR: https://git.openjdk.java.net/jdk/pull/1679
