Skip to content

Commit 45018a8

Browse files
livecodealilivecodeali
committed
[[ Bug 19911 ]] Ensure android interface callbacks are on engine thread
This patch changes the thread affinity specification in the binding string to use an optional `?thread` clause at the end of the string.
1 parent fa8cdcf commit 45018a8

File tree

9 files changed

+116
-40
lines changed

9 files changed

+116
-40
lines changed

‎docs/guides/LiveCode Builder Language Reference.md‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -557,8 +557,8 @@ non-Windows platform then it is taken to be `default`.
557557

558558
The Java binding string has the following form:
559559

560-
"java:[className>][functionType.]function[!calling]"
561-
560+
"java:[className>][functionType.]function[!calling][?thread]"
561+
562562
Here *className* is the qualified name of the Java class to bind to.
563563

564564
Here *functionType* is either empty, or `get` or `set`, which are
@@ -636,7 +636,12 @@ or
636636
_JNI_SetMouseListener(pJButton, tListener)
637637
end unsafe
638638
end handler
639-
639+
640+
>*Important:* On Android, interface callbacks are *always* run on the
641+
> engine thread. This means JNI local references from other threads
642+
> (in particular the UI thread) are unavailable. Therefore it is not
643+
> advised to do anything using the JNI in interface callbacks.
644+
640645
Here *calling* specifies the calling convention which can be one of:
641646

642647
- `instance`
@@ -647,6 +652,9 @@ Instance and nonvirtual calling conventions require instances of the given
647652
Java class, so the foreign handler declaration will always require a Java
648653
object parameter.
649654

655+
Here, *thread* is either empty or `ui`. The `ui` form is used on
656+
Android when binding to methods that must be run on the UI thread.
657+
650658
> **Warning:** At the moment it is not advised to use callbacks that may be
651659
> executed on arbitrary threads, as this is likely to cause your application
652660
> to crash.

‎docs/lcb/notes/19911.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# LiveCode Builder Language
2+
3+
## Android Listener support
4+
5+
# [19911] Ensure interface callbacks are executed on engine thread
6+
7+
## Foreign Function Interface
8+
9+
* It is now possible to specify the thread to be used in Java-bound foreign handlers. This is done by appending `?<thread>` to the end of the binding string. Currently the only supported value is `ui`, for running java on the android UI thread. This has no effect on desktop platforms.

‎engine/src/mblandroiddc.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1956,6 +1956,11 @@ bool MCAndroidIsOnSystemThread(void)
19561956
return s_android_ui_thread.IsCurrent();
19571957
}
19581958

1959+
bool MCAndroidIsOnEngineThread(void)
1960+
{
1961+
return s_android_engine_thread.IsCurrent();
1962+
}
1963+
19591964
////////////////////////////////////////////////////////////////////////////////
19601965

19611966
extern "C" JNIEXPORT void JNICALL Java_com_runrev_android_Engine_doCreate(JNIEnv *env, jobject object, jobject activity, jobject container, jobject view) __attribute__((visibility("default")));

‎engine/src/mblandroidlcb.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,13 @@ extern "C" JNIEXPORT void JNICALL Java_com_runrev_android_LCBInvocationHandler_d
5555
JNIEXPORT void JNICALL Java_com_runrev_android_LCBInvocationHandler_doNativeListenerCallback(JNIEnv *env, jobject object, jlong p_handler, jstring p_method_name, jobjectArray p_args)
5656
{
5757
#ifdef TARGET_SUBPLATFORM_ANDROID
58-
extern bool MCAndroidIsOnSystemThread(void);
59-
if (!MCAndroidIsOnSystemThread())
58+
extern bool MCAndroidIsOnEngineThread(void);
59+
if (!MCAndroidIsOnEngineThread())
6060
{
6161
typedef void (*co_yield_callback_t)(void *);
62-
extern void co_yield_to_android_and_call(co_yield_callback_t callback, void *context);
62+
extern void co_yield_to_engine_and_call(co_yield_callback_t callback, void *context);
6363
remote_call_t t_context = {p_handler, p_method_name, p_args};
64-
co_yield_to_android_and_call(remote_call_func, &t_context);
64+
co_yield_to_engine_and_call(remote_call_func, &t_context);
6565
}
6666
else
6767
{

‎extensions/widgets/androidbutton/androidbutton.lcb‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,19 @@ foreign handler _JNI_GetAndroidEngine() returns JObject binds to "java:com.runre
9494
foreign handler _JNI_GetEngineContext(in pEngine as JObject) returns JObject binds to "java:android.view.View>getContext()Landroid/content/Context;"
9595

9696
// Handlers for creating and attaching view
97-
foreign handler _JNI_CreateButton(in pContext as JObject) returns JObject binds to "javaui:android.widget.Button>new(Landroid/content/Context;)"
98-
foreign handler _JNI_AddButtonView(in pParentView as JObject, in pChildView as JObject) returns nothing binds to "javaui:android.view.ViewGroup>addView(Landroid/view/View;)V"
97+
foreign handler _JNI_CreateButton(in pContext as JObject) returns JObject binds to "java:android.widget.Button>new(Landroid/content/Context;)?ui"
98+
foreign handler _JNI_AddButtonView(in pParentView as JObject, in pChildView as JObject) returns nothing binds to "java:android.view.ViewGroup>addView(Landroid/view/View;)V?ui"
9999

100100
// Handlers for adding click listener
101101
handler type ClickCallback(in pView as JObject)
102102
foreign handler _JNI_OnClickListener(in pHandler as ClickCallback) returns JObject binds to "java:android.view.View$OnClickListener>interface()"
103-
foreign handler _JNI_SetOnClickListener(in pButton as JObject, in pListener as JObject) returns nothing binds to "javaui:android.view.View>setOnClickListener(Landroid/view/View$OnClickListener;)V"
103+
foreign handler _JNI_SetOnClickListener(in pButton as JObject, in pListener as JObject) returns nothing binds to "java:android.view.View>setOnClickListener(Landroid/view/View$OnClickListener;)V?ui"
104104

105105
// Property setters
106-
foreign handler _JNI_SetTextViewText(in pView as JObject, in pValue as JString) returns nothing binds to "javaui:android.widget.TextView>setText(Ljava/lang/CharSequence;)V"
107-
foreign handler _JNI_SetTextViewTextColor(in pView as JObject, in pValue as JInt) returns nothing binds to "javaui:android.widget.TextView>setTextColor(I)V"
106+
foreign handler _JNI_SetTextViewText(in pView as JObject, in pValue as JString) returns nothing binds to "java:android.widget.TextView>setText(Ljava/lang/CharSequence;)V?ui"
107+
foreign handler _JNI_SetTextViewTextColor(in pView as JObject, in pValue as JInt) returns nothing binds to "java:android.widget.TextView>setTextColor(I)V?ui"
108108
foreign handler _JNI_GetColorFromARGB(in pA as JInt, in pR as JInt, in pG as JInt, in pB as JInt) returns JInt binds to "java:android.graphics.Color>argb(IIII)I!static"
109-
foreign handler _JNI_SetTextViewEnabled(in pView as JObject, in pValue as JBoolean) returns nothing binds to "javaui:android.view.View>setEnabled(Z)V"
109+
foreign handler _JNI_SetTextViewEnabled(in pView as JObject, in pValue as JBoolean) returns nothing binds to "java:android.view.View>setEnabled(Z)V?ui"
110110

111111
private variable mLabel as String
112112
private variable mColor as String

‎libscript/src/script-error.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,15 @@ MCScriptThrowUnableToLoadForiegnLibraryError(void)
351351
nil);
352352
}
353353

354+
bool
355+
MCScriptThrowUnknownJavaThreadError(void)
356+
{
357+
return MCErrorCreateAndThrow(kMCGenericErrorTypeInfo,
358+
"reason",
359+
MCSTR("unknown thread for java binding"),
360+
nil);
361+
}
362+
354363
bool
355364
MCScriptThrowJavaBindingNotImplemented(void)
356365
{

‎libscript/src/script-execute.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ class MCScriptForeignInvocation
172172
case kMCScriptForeignHandlerLanguageJava:
173173
#ifdef TARGET_SUBPLATFORM_ANDROID
174174
extern bool MCAndroidIsOnSystemThread();
175-
if (!p_handler->java.is_ui_bound || MCAndroidIsOnSystemThread())
175+
if (p_handler->java.thread != kMCJavaThreadUI || MCAndroidIsOnSystemThread())
176176
#endif
177177
{
178178
MCJavaCallJNIMethod(p_handler->java.class_name,

‎libscript/src/script-instance.cpp‎

Lines changed: 62 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,24 @@ static MCJavaCallType __MCScriptGetJavaCallType(MCStringRef p_class, MCStringRef
694694
return MCJavaCallTypeUnknown;
695695
}
696696

697+
// Resolve the call type
698+
static bool __MCScriptGetJavaThread(MCStringRef p_thread, MCJavaThread &r_thread)
699+
{
700+
if (MCStringIsEmpty(p_thread))
701+
{
702+
r_thread = kMCJavaThreadDefault;
703+
return true;
704+
}
705+
706+
if (MCStringIsEqualToCString(p_thread, "ui", kMCStringOptionCompareCaseless))
707+
{
708+
r_thread = kMCJavaThreadUI;
709+
return true;
710+
}
711+
712+
return MCScriptThrowUnknownJavaThreadError();
713+
}
714+
697715
static bool
698716
__MCScriptResolveForeignFunctionBindingForC(MCScriptInstanceRef p_instance,
699717
MCScriptForeignHandlerDefinition *p_handler,
@@ -832,35 +850,60 @@ static bool
832850
__MCScriptResolveForeignFunctionBindingForJava(MCScriptInstanceRef p_instance,
833851
MCScriptForeignHandlerDefinition *p_handler,
834852
/* takes */ MCStringRef d_binding,
835-
bool p_is_ui,
836853
bool* r_bound)
837854
{
838855
MCAutoStringRef t_library;
839856
MCAutoStringRef t_class;
840857
MCAutoStringRef t_function_string;
841858
MCAutoStringRef t_calling;
842-
if (!__MCScriptSplitForeignBindingString(d_binding,
843-
'>',
844-
&t_library) ||
845-
!__MCScriptSplitForeignBindingString(d_binding,
846-
'.',
847-
&t_class) ||
848-
!MCStringDivideAtChar(d_binding,
849-
'!',
850-
kMCStringOptionCompareExact,
851-
&t_function_string,
852-
&t_calling))
853-
{
854-
MCValueRelease(d_binding);
855-
return false;
856-
}
859+
MCAutoStringRef t_thread;
860+
861+
bool t_success =
862+
__MCScriptSplitForeignBindingString(d_binding,
863+
'>',
864+
&t_library) &&
865+
__MCScriptSplitForeignBindingString(d_binding,
866+
'.',
867+
&t_class) &&
868+
__MCScriptSplitForeignBindingString(d_binding,
869+
'!',
870+
&t_function_string);
871+
872+
MCAutoStringRef t_to_divide;
873+
if (t_success && !MCStringIsEmpty(*t_function_string))
874+
{
875+
// Calling convention is not empty
876+
t_success = MCStringDivideAtChar(d_binding,
877+
'?',
878+
kMCStringOptionCompareExact,
879+
&t_calling,
880+
&t_thread);
881+
}
882+
else if (t_success)
883+
{
884+
MCAutoStringRef t_real_function;
885+
// Calling convention is empty
886+
t_success = MCStringDivideAtChar(d_binding,
887+
'?',
888+
kMCStringOptionCompareExact,
889+
&t_real_function,
890+
&t_thread);
891+
t_function_string.Reset(*t_real_function);
892+
t_calling = kMCEmptyString;
893+
}
894+
857895
MCValueRelease(d_binding);
858-
896+
if (!t_success)
897+
return false;
898+
859899
MCAutoStringRef t_arguments, t_return, t_function;
860900
if (!__split_function_signature(*t_function_string, &t_function, &t_arguments, &t_return))
861901
return false;
862902

863-
p_handler -> java . is_ui_bound = p_is_ui;
903+
MCJavaThread t_java_thread;
904+
if (!__MCScriptGetJavaThread(*t_thread, t_java_thread))
905+
return false;
906+
p_handler->java.thread = t_java_thread;
864907

865908
p_handler -> java . call_type = __MCScriptGetJavaCallType(*t_class,
866909
*t_function,
@@ -972,17 +1015,11 @@ __MCScriptResolveForeignFunctionBinding(MCScriptInstanceRef p_instance,
9721015
t_rest,
9731016
r_bound);
9741017
}
975-
else if (MCStringIsEqualToCString(*t_language, "java", kMCStringOptionCompareExact) ||
976-
MCStringIsEqualToCString(*t_language, "javaui", kMCStringOptionCompareExact))
1018+
else if (MCStringIsEqualToCString(*t_language, "java", kMCStringOptionCompareExact))
9771019
{
978-
bool t_is_ui_bound;
979-
t_is_ui_bound = MCStringIsEqualToCString(*t_language,
980-
"javaui",
981-
kMCStringOptionCompareExact);
9821020
return __MCScriptResolveForeignFunctionBindingForJava(p_instance,
9831021
p_handler,
9841022
t_rest,
985-
t_is_ui_bound,
9861023
r_bound);
9871024
}
9881025

‎libscript/src/script-private.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ enum MCScriptForeignHandlerLanguage
357357
kMCScriptForeignHandlerLanguageJava,
358358
};
359359

360+
enum MCJavaThread {
361+
kMCJavaThreadDefault,
362+
kMCJavaThreadUI
363+
};
364+
360365
struct MCScriptForeignHandlerDefinition: public MCScriptCommonHandlerDefinition
361366
{
362367
MCStringRef binding;
@@ -379,7 +384,7 @@ struct MCScriptForeignHandlerDefinition: public MCScriptCommonHandlerDefinition
379384
MCNameRef class_name;
380385
void *method_id;
381386
int call_type : 8;
382-
bool is_ui_bound : 1;
387+
int thread : 8;
383388
} java;
384389
};
385390
};
@@ -652,6 +657,9 @@ MCScriptThrowJavaBindingNotImplemented(void);
652657
bool
653658
MCScriptThrowJavaBindingNotSupported(void);
654659

660+
bool
661+
MCScriptThrowUnknownJavaThreadError(void);
662+
655663
bool
656664
MCScriptCreateErrorExpectedError(MCErrorRef& r_error);
657665

0 commit comments

Comments
 (0)