Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Last tweaks for Android #1851

Merged
dlang-bot merged 3 commits into from
Jun 25, 2017
Merged

Last tweaks for Android #1851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
639c90c
Disable recursive mutex destruction test on Android.
joakim-noah Jun 24, 2017
092e942
Swap GC signals on Android, because the Java VM intercepts SIGUSR1 an…
joakim-noah Jun 24, 2017
36fcc58
Correct a typo in the TLS delimiter name and rewrite an incorrect com…
joakim-noah Jun 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/core/sync/mutex.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,9 @@ unittest
// by checking that locking is not possible. This assumes
// that the underlying implementation is well behaved
// and makes the object non-lockable upon destruction.
// For example, Bionic doesn't appear to do so, so this test is
// not run on Android.
version (CRuntime_Bionic) {} else
Copy link
Contributor Author

@joakim-noah joakim-noah Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test asserts on Android 6.0 Marshmallow, as it doesn't appear that pthread_mutex_destroy really destroys the mutex there. I notice that if I comment out the line that sets PTHREAD_MUTEX_RECURSIVE in this druntime module, the test passes again, but I haven't dug into why it only doesn't work for recursive mutexes. It doesn't, so this test can't be run as is.

Copy link
Member

@PetarKirov PetarKirov Jun 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the author of this test (and the test below) I think it might have been a mistake to rely on implementation defined behavior here (actually undefined behavior according to the Posix standard). I assumed that pthread_mutex_trylock would return EINVAL on all platforms if called with a previously destroyed mutex (otherwise what would be the point of specifying such return value in the standard?).

I would love to find a better way to verify that the internal implementation has been destroyed successfully, but I can't think of any other alternatives at the moment.

assert(!mtx.tryLock_nothrow());

free(cast(void*) mtx);
Expand Down
4 changes: 4 additions & 0 deletions src/core/thread.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,10 @@ extern (C) void thread_init()
// functions to detect the condition and return immediately.

Thread.initLocks();
// The Android VM runtime intercepts SIGUSR1 and apparently doesn't allow
// its signal handler to run, so swap the two signals on Android, since
// thread_resumeHandler does nothing.
version( Android ) thread_setGCSignals(SIGUSR2, SIGUSR1);
Copy link
Contributor Author

@joakim-noah joakim-noah Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Android VM runtime intercepts SIGUSR1 in its signal-catcher thread and apparently doesn't run its signal-handler. This doesn't matter when D executables are run as standalone processes on the Android command-line, but it causes problems when threads are suspended in an Android app that runs D code in a shared library called from the VM runtime, as that's how native code normally has to be run on Android.

When the unit tests for core.thread are run in an Android app without this change, they hang at pthread_kill when it tries to suspend a thread. My guess is this is because the VM runtime intercepts SIGUSR1 and the thread_suspendHandler is never run. If I swap the two signals, it still resumes fine, presumably because the thread_resumeHandler does nothing, so it doesn't matter if it doesn't run.

I've been using Ilya's #1565 also, which works fine. However, that pull has been stuck in limbo for more than a year, so I'd like to get this in in the meantime. Whenever that pull is approved, this can be removed.


version( Darwin )
{
Expand Down
13 changes: 7 additions & 6 deletions src/rt/sections_android.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void initSections() nothrow @nogc
auto mend = cast(immutable ModuleInfo**)&__stop_minfo;
_sections.moduleGroup = ModuleGroup(mbeg[0 .. mend - mbeg]);

auto pbeg = cast(void*)&_tls_end;
auto pbeg = cast(void*)&_tlsend;
auto pend = cast(void*)&__bss_end__;
_sections._gcRanges[0] = pbeg[0 .. pend - pbeg];
}
Expand All @@ -93,11 +93,12 @@ void scanTLSRanges(void[]* rng, scope void delegate(void* pbeg, void* pend) noth
dg(rng.ptr, rng.ptr + rng.length);
}

/* NOTE: The Bionic C library does not allow storing thread-local data
* in the normal .tbss/.tdata ELF sections. So instead we roll our
* own by simply putting tls into the non-tls .data/.bss sections
* and using the _tlsstart/_tlsend symbols as delimiters of the tls
* data.
/* NOTE: The Bionic C library ignores thread-local data stored in the normal
* .tbss/.tdata ELF sections, which are marked with the SHF_TLS/STT_TLS
* flags. So instead we roll our own by keeping TLS data in the
* .tdata/.tbss sections but removing the SHF_TLS/STT_TLS flags, and
* access the TLS data using this function and the _tlsstart/_tlsend
* symbols as delimiters.
*
* This function is called by the code emitted by the compiler. It
* is expected to translate an address in the TLS static data to
Expand Down