Disable main_thread_id assertion for Android debug build

[jeyum2]

This assertion block was previously commented out and reactivated without a clear reason in git history
It causes a runtime panic in debug builds for Android
If this assertion is required, please implement logic that also considers Android

[Bromeon]

Thank you! Could you add a comment re. why it's commented-out? To prevent this from happening again.

Please then squash commits to 1 🙂

[jeyum2]

The assertion was revived on 82a14f68

I’m not sure about the current status of Linux hot-reloading.
If it works on Linux, I will just add a comment for Android.
If not, I will restore the original comment with an additional comment for Android.

[Bromeon]

What about expanding the if cfg!(...) condition to include a Linux or Android test?

Followed by an explanation why on Android this doesn't work, that would be really valuable.

[jeyum2]

I have added checking Android to the existing cfg condition
In this PR, we focus only on Android and suggest not modifying the previous code

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-780

[Bromeon]

There's a problem with this change, which is explained in the assert message:

attempted to access binding from different thread than main thread; this is UB

The whole rest of the library relies on access being single-threaded -- if we just disable the check, we might really run into UB.

---

Does anyone familiar with Android know why access happens from a different thread in Android? And if that's fine, or what the best practices are to deal with this?

Maybe one of the people involved in #470, e.g. @scripturial @moberer @MeganSpencer @Jim-Bar @walksanatora?

[Bromeon]

You probably mean TODO: instead of CHECK?

[scripturial]

The whole rest of the library relies on access being single-threaded -- if we just disable the check, we might really run into UB.

Does anyone familiar with Android know why access happens from a different thread in Android? And if that's fine, or what the best practices are to deal with this?

I don’t feel qualified to comment on Godot threading issues but I do have a suspicion regarding the cause of the problem I had. I didn’t realise callbacks from http requests in Godot could be on a separate thread. (I assume this is the case.) so when crossing into rust it’s possible that you end up in rust on a different thread (without realising). I think this can then cause updates to data or callbacks into Godot to occasionally crash.

The work around for me has to call back from rust into Godot using ‘call_deferred’ I’m not sure if it is the design of call_deferred to force “cross to main thread” but since I started doing this I’ve not had any android crashes.

The Swift plugin developer that made one of my plugins does the same “hack” where you call back into a gdscript function then invoke call_deferred which makes it rather messy.

It may be that this patch actually would have told me what I did wrong straight away and helped me solve the problem quicker. So maybe it’s a win?

(On hot reloading specifically, I use Mac for development so I can’t comment on hot reloading specifically)

[Bromeon]

@scripturial Thanks for the info!

The Swift plugin developer that made one of my plugins does the same “hack” where you call back into a gdscript function then invoke call_deferred which makes it rather messy.

Could you link me to that plugin and/or issue they had?

I don’t feel qualified to comment on Godot threading issues but I do have a suspicion regarding the cause of the problem I had. I didn’t realise callbacks from http requests in Godot could be on a separate thread. (I assume this is the case.) so when crossing into rust it’s possible that you end up in rust on a different thread (without realising). I think this can then cause updates to data or callbacks into Godot to occasionally crash.

Do you have any suspicion why this might happen on Android but not on other platforms?

---

Or maybe @jeyum2, can you be more specific about the circumstances which caused panic in your case? Can you come up with a minimal, complete example that reproduces the problem?

[Bromeon]

I've just merged #794 which addressed a similar issue, with Wasm. Could you rebase the PR onto latest master and resolve the conflicts?

Also, would be great to hear about the panic circumstances 🙂

Or maybe @jeyum2, can you be more specific about the circumstances which caused panic in your case? Can you come up with a minimal, complete example that reproduces the problem?

Thank you!

[jeyum2]

@scripturial Thanks for the info!

The Swift plugin developer that made one of my plugins does the same “hack” where you call back into a gdscript function then invoke call_deferred which makes it rather messy.

Could you link me to that plugin and/or issue they had?

I don’t feel qualified to comment on Godot threading issues but I do have a suspicion regarding the cause of the problem I had. I didn’t realise callbacks from http requests in Godot could be on a separate thread. (I assume this is the case.) so when crossing into rust it’s possible that you end up in rust on a different thread (without realising). I think this can then cause updates to data or callbacks into Godot to occasionally crash.

Do you have any suspicion why this might happen on Android but not on other platforms?

Or maybe @jeyum2, can you be more specific about the circumstances which caused panic in your case? Can you come up with a minimal, complete example that reproduces the problem?

When running a debug build of an empty project, a panic occurs immediately during the initialization process.
This is the same issue as discussed in this Discord thread

03-17 18:19:18.394 16927 16927 F DEBUG   :       #01 pc 0000000000624564  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (std::sys::unix::abort_internal::h98c5b5a7186aa89f+4)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #02 pc 0000000000622734  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (rust_panic+88)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #03 pc 0000000000622508  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (std::panicking::rust_panic_with_hook::hda05105b262e03ce+664)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #04 pc 0000000000622238  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::h5062df77b9a38169+148)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #05 pc 0000000000621004  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (std::sys_common::backtrace::__rust_end_short_backtrace::h1cde41c1fc685d95+4)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #06 pc 0000000000621fd8  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (rust_begin_unwind+56)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #07 pc 000000000063ce5c  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (core::panicking::panic_fmt::hc62e9e70e1db8b93+44)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #08 pc 000000000063d1d0  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (core::panicking::assert_failed_inner::h1e933bb60f07571f+272)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #09 pc 00000000005fed70  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (core::panicking::assert_failed::hc8084b7fe27edcd5+52)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #10 pc 00000000003198ec  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (godot_core::private::handle_panic_with_print::h767f77e15a6bd188+2224)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #11 pc 0000000000314d9c  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (godot_core::private::handle_panic::h1762d4504f5cad13+120)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #12 pc 00000000002e35a0  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libtarnish_rust.so (godot_core::init::ffi_initialize_layer::ha2156e5340c87084+64)
03-17 18:19:18.394 16927 16927 F DEBUG   :       #13 pc 0000000003816c24  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libgodot_android.so
03-17 18:19:18.394 16927 16927 F DEBUG   :       #14 pc 0000000003837850  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libgodot_android.so
03-17 18:19:18.394 16927 16927 F DEBUG   :       #15 pc 0000000000e55538  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libgodot_android.so
03-17 18:19:18.394 16927 16927 F DEBUG   :       #16 pc 0000000000e2de68  /data/app/~~Ks3JY4pqes9m0gwzSWYDQw==/org.godotengine.tarnish-7ALG45aOnZwqwsvmqkQjBQ==/lib/arm64/libgodot_android.so (Java_org_godotengine_godot_G```

[jeyum2]

I have confirmed that the same panic occurs on Android with the master branch where #794 has been merged.
I have rebased and modified the code to add 'android' to the cfg flag.

[Bromeon]

While I'm not a fan of not getting to the root of things and just silencing the problem, it doesn't help anyone if Android setup is just totally broken, either. I added a comment in the code, hopefully someone with more expertise can investigate this in the future.