On Fri, 18 Dec 2020 22:26:45 GMT, Phil Race <p...@openjdk.org> wrote:
>>> > I guess it should work already in the JNF case? The java exception should >>> > be wrapped by the NSException in the JNFCallObjectMethod/etc and >>> > unwrapped in the JNF_COCOA_ENTER before exists to the java method? >>> >>> That is the pattern I know is there today and I don't think is needed. >>> It can be done but I don't think it needed. I'd prefer to see if we ever >>> have such a situation and deal with it "locally" >>> at the call site. >> >> No, it is quite useful, and it will be good to preserve it. It is possible >> to work "locally" when the call site if come from java, but not if we are in >> the infinite native loop. >> >>> > > So I believe we may end up more (theoretically) robust with my changes >>> > > than we were before. >>> > > Perhaps they weren't a result of a JNF* call but still it seems >>> > > improved. >>> > I am not sure about that. >>> I am looking at JNI warnings - not just guessing. >> >> I am looking at a new code that is not as good as before. > >> > > I guess it should work already in the JNF case? The java exception >> > > should be wrapped by the NSException in the JNFCallObjectMethod/etc and >> > > unwrapped in the JNF_COCOA_ENTER before exists to the java method? >> > >> > >> > That is the pattern I know is there today and I don't think is needed. >> > It can be done but I don't think it needed. I'd prefer to see if we ever >> > have such a situation and deal with it "locally" >> > at the call site. >> >> No, it is quite useful, and it will be good to preserve it. It is possible >> to work "locally" when the call site if come from java, but not if we are in >> the infinite native loop. > > an NS exception on the appkit will terminate the app - if unhandled - and > probably will even if handled > so I am not sure what is better about that > >> >> > > > So I believe we may end up more (theoretically) robust with my changes >> > > > than we were before. >> > > > Perhaps they weren't a result of a JNF* call but still it seems >> > > > improved. >> > > > I am not sure about that. >> > > > I am looking at JNI warnings - not just guessing. >> >> I am looking at a new code that is not as good as before. > > I guess we disagree. I think this is better and simpler. Not trying to prolong or expand this discussion but a 1:1 mapping to JNF to really minimise the review needs to account for 1) We aren't going to call it "JNF" 2) The lookup methods for members is quirky in that it is the same for methods and fields and I can only suppose that the signature is parsed to look for "(...)" to decide which it should be 3) The calling of static methods (and look up of static field values) doesn't take the class as an argument so there's some complexity in there just to support that which I think un-needed 4) We'd need a bunch more macros to handle all the same "Call" cases as JNF has and that seems un-neccessary too. 5) There are already a substantial number of cases where we don't use JNF 6) So long as we have them in place the CHECK calls are simpler and if we ever reallly wanted to we could have a variant of it that does the odd Java->NS->Java exception think. Or just change the behaviour of the current one. But I stand by what I wrote that I don't think this is needed. This is code which doesn't get exercised - it is more like InternalError than IOException handling. ------------- PR: https://git.openjdk.java.net/jdk/pull/1679
