Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Warn unimplemented loadLibrary on Android #206

Open
wants to merge 1 commit into
base: ldc
Choose a base branch
from

Conversation

MrcSnm
Copy link

@MrcSnm MrcSnm commented Nov 16, 2022

rt_loadLibrary and rt_unloadLibrary are unimplemented for Android. I don't know if this compiles because I can't compile it myself, but the idea should be fairly straightforward.

I don't know about the build process of having pragma(lib) in runtime, but maybe pragma(lib, "android"); Could be required

rt_loadLibrary and rt_unloadLibrary are unimplemented for Android. I don't know if this compiles because I can't compile it myself, but the idea should be fairly straightforward.

I don't know about the build process of having pragma(lib) in runtime, but maybe pragma(lib, "android"); Could be required
@kinke
Copy link
Member

kinke commented Nov 17, 2022

After a quick look, they aren't implemented for Posix systems without shared-druntime support, so should yield a linker error when trying to use them (directly or indirectly). I think that's better than a runtime error, although a compile error would be best (but doesn't look feasible in this instance, for such a druntime-internal helper).

@kinke
Copy link
Member

kinke commented Nov 17, 2022

Well, it's not a runtime error, just a logged warning, even worse IMO. ;) - To be clear: this problem isn't really Android-specific (although it's one of the few platforms where LDC doesn't support a druntime .so, shared by all D binaries of the process - and getting that to work would probably require a total overhaul of our Android TLS emulation).

And it's only a problem when trying to load shared D libraries, as that requires extra work (registering TLS ranges with the GC, running module constructors etc.), where a dlopen alone isn't sufficient and is guaranteed to cause serious problems at some point. So when loading .so libs not containing D parts on Android, it's probably better to dlopen them directly, not going through Runtime.loadLibrary().

@JohanEngelen
Copy link
Member

Agree with kinke. Changing the linker error into a runtime warning hides what would be a very serious bug. Better to keep the link error and let the user himself choose to use dlopen (and introduce the bug with D libs).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants