lambdanative icon indicating copy to clipboard operation
lambdanative copied to clipboard

ANDROID: additional error checking on JNI an more

Open 0-8-15 opened this issue 5 years ago • 8 comments

Preface: I have not been able to break this in meaningful smaller patches. All changes where required to pass a meaningful test case.

1.) Android 9/10 restricts access to JNI and Jave reflection API's.

This adds additional checks.

Commented out an possible optimization, which could be assumed to works, but does not for me so far. Please leave this comment in, so we recall before we wonder why and try over and over again. Once in a while we should ponder if it still does not work or why.

2.) Clear Java exceptions.

This enables to continue to run when an exception ocured during a JNI call. (Future versions shall forward this as an exception in Gambit.)

3.) Avoid some JNI calls and provide more information about the Java/Android environment to Scheme.

This is required for (upcoming) tricks to still call embedded dynamic libraries as subprocesses.

It also enables to figure out a sane path to store app-private data instead of the deprecated hard coding of the publicly accessible path /sdcard.

4.) Add support to switch the apps content view to Java and back.

An upcoming module webview will need this.

0-8-15 avatar Nov 02 '20 11:11 0-8-15

I might break this into three commit when I push it into master, just to clearly identify what does what. Not sure yet though.

mgorges avatar Nov 05 '20 19:11 mgorges

Am Thu, 05 Nov 2020 11:28:33 -0800 schrieb Matthias Görges [email protected]:

I might break this into three commit when I push it into master, just to clearly identify what does what. Not sure yet though.

I don't mind. I just did not know how to test one of the changes without the other being avail as well.

0-8-15 avatar Nov 06 '20 17:11 0-8-15

Am Sat, 07 Nov 2020 01:28:15 -0800 schrieb Matthias Görges [email protected]:

@mgorges commented on this pull request.

return (error?NULL:env);
}

+int JNI_forward_exception_to_gambit(JNIEnv*env) {

  • // TBD: actually forward, not only clear!
  • if((*env)->ExceptionCheck(env)) {
  • (*env)->ExceptionClear(env);

So a call into log_c with content as described in https://stackoverflow.com/questions/10408972/how-to-obtain-a-description-of-a-java-exception-in-c-when-using-jni ?

Either this. Though I've been more thinking of extracting info about the exception and raise an exception in the Scheme world.

0-8-15 avatar Nov 07 '20 10:11 0-8-15

Am Sat, 07 Nov 2020 00:59:47 -0800 schrieb Matthias Görges [email protected]:

@mgorges commented on this pull request.

@@ -94,4 +94,18 @@ end-of-c-declare
(gambit-c (if (string=? (system-platform) "android") (##heartbeat-interval-set! -1.))) (else (if (string=? (system-platform) "android") (##set-heartbeat-interval! -1.)))) +(cond-expand

  • (android
  • (c-declare #<<EOF +extern char* android_getFilesDir_info_get(); +char* android_getFilesDir_info() +{
  • return android_getFilesDir_info_get(); +} +extern char* android_getPackageCodePath(); +EOF +)
  • (define (android-PackageCodePath) ((c-lambda () char-string "android_getPackageCodePath"))))

Also this function is not defined on non-android platforms, which may be a problem as how do we prevent users from including this in their code? Wouldn't it be safer to always declare it and simply not do anything on non-android platforms?

No real preference about that here.

Though I'd tend to not pollute the toplevel namespace with things likely to be changed again as soon as goggle think it's time to change.

Also: something it will have to return. Which value? Why? Eventually useful it's going to be on Android only anyway.

0-8-15 avatar Nov 07 '20 10:11 0-8-15

Am Sat, 07 Nov 2020 00:54:15 -0800 schrieb Matthias Görges [email protected]:

@mgorges commented on this pull request.

@@ -94,4 +94,18 @@ end-of-c-declare
(gambit-c (if (string=? (system-platform) "android") (##heartbeat-interval-set! -1.))) (else (if (string=? (system-platform) "android") (##set-heartbeat-interval! -1.)))) +(cond-expand

  • (android
  • (c-declare #<<EOF +extern char* android_getFilesDir_info_get(); +char* android_getFilesDir_info()

You are now making a third way of calling android_getFilesDir() - am I missing something that makes this one different? as otherwise only the extern char* android_getPackageCodePath() needs to stay here?

That's true. This getFileDir_info_get() is a leftover from my experiments. Overlooked. Should have been deleted before submission.

0-8-15 avatar Nov 07 '20 10:11 0-8-15

Alright, I hope I have faithfully committed this as three separate commits. I then had to fix (*env)->ReleaseStringUTFChars(env, codePath, NULL); as I get a JNI exception with NULL not being allowed? Will need to do a bit more testing but the simple apps I tried in the simulator seem to run.

mgorges avatar Nov 09 '20 12:11 mgorges

Short of the dealing of JNI exceptions, has this been merged in functionally equivalent three changes so we can close this one?

mgorges avatar Nov 20 '20 17:11 mgorges

let's see.

give me some to reconcile my differences here

0-8-15 avatar Nov 20 '20 20:11 0-8-15