Skip to content

Thread detach hook for Java JNI on Android #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed

Thread detach hook for Java JNI on Android #250

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c3c0bef
Thread detach hook for Java JNI on Android
johnno1962 May 22, 2017
96bc6b6
Thread detach hook for Java JNI on Android II
johnno1962 May 23, 2017
b6003ed
Thread detach hook for Java JNI on Android III
johnno1962 May 23, 2017
8bf5bcf
Renamed for consistency
johnno1962 May 25, 2017
bd54618
Revert public API change
johnno1962 May 26, 2017
4be52da
Move to queue_private.h
johnno1962 May 27, 2017
10ea8cf
Implemented getter
johnno1962 May 27, 2017
12163ba
Single entry point
johnno1962 May 27, 2017
0a57226
Setter only
johnno1962 May 28, 2017
39eddf2
Thread detach hook for Java JNI on Android
johnno1962 May 22, 2017
41772bd
Merge branch 'master' of https://github.com/Concoction/swift-corelibs…
johnno1962 Jun 20, 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
11 changes: 11 additions & 0 deletions private/queue_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,17 @@ void
dispatch_async_enforce_qos_class_f(dispatch_queue_t queue,
void *_Nullable context, dispatch_function_t work);

#ifdef __ANDROID__
/*!
* @handler _dispatch_thread_detach_callback_ptr
*
* Hook to be able to detach threads from the Java JVM before they exit.
* If JNI has been used on a thread on Android it needs to have been
* "detached" before the thread exits or the application will crash.
*/
DISPATCH_EXPORT
void *(*_dispatch_thread_detach_callback_ptr(void))(void);
#endif

__END_DECLS

Expand Down
14 changes: 14 additions & 0 deletions src/queue.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,15 @@ gettid(void)
if ((f) && tsd->k) ((void(*)(void*))(f))(tsd->k); \
} while (0)

#ifdef __ANDROID__
static void (*_dispatch_thread_detach_callback)(void);
Copy link
Contributor

Choose a reason for hiding this comment

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

if it's static then the overlay which is a dynamic library can't pick it up.

@das suggested to me the right place is to export the symbol is from private/queue_private.h or private/private.h and also to use silgen like you did.

Copy link
Contributor Author

@johnno1962 johnno1962 May 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Global symbol is now a function _dispatch_set_detach_callback_ptr as it doesn’t seem possible to express a global var in a silgen. I’ve moved it's declaration into queue_private.h.


void
*(*_dispatch_thread_detach_callback_ptr(void))(void) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is hard to read, also exposing a pointer to a function pointer is really weird. do you really need the getter? it feels like you only want to install it, I would have gone for:

void
_dispatch_install_thread_detach_callback(dispatch_function_t cb)
{
    if (os_atomic_xchg(&_dispatch_thread_detach_callback, cb, relaxed)) {
        DISPATCH_CLIENT_CRASH(0, "Installing a thread detach callback twice");
    }
}

return &_dispatch_thread_detach_callback;
}
#endif

void
_libdispatch_tsd_cleanup(void *ctx)
{
Expand All @@ -885,6 +894,11 @@ _libdispatch_tsd_cleanup(void *ctx)
_tsd_call_cleanup(dispatch_voucher_key, _voucher_thread_cleanup);
_tsd_call_cleanup(dispatch_deferred_items_key,
_dispatch_deferred_items_cleanup);
#ifdef __ANDROID__
if (_dispatch_thread_detach_callback) {
_dispatch_thread_detach_callback();
}
#endif
tsd->tid = 0;
}

Expand Down
14 changes: 14 additions & 0 deletions src/swift/Queue.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,20 @@ public extension DispatchQueue {
let p = Unmanaged.passRetained(v).toOpaque()
dispatch_queue_set_specific(self.__wrapped, k, p, _destructDispatchSpecificValue)
}

#if os(Android)
@_silgen_name("_dispatch_thread_detach_callback_ptr")
private static func _dispatch_thread_detach_callback_ptr() -> UnsafeMutablePointer<(@convention(c) () -> Void)?>

public static var threadDetachCallback: (@convention(c) () -> Void)? {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think only a one-way setter should be exposed. having a getter makes the interface un-necessarily ugly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the potiners. It helps me zero in on something you’re happy with. Changed pushed.

get {
return _dispatch_thread_detach_callback_ptr().pointee
}
set(newValue) {
_dispatch_thread_detach_callback_ptr().pointee = newValue
}
}
#endif
}

private func _destructDispatchSpecificValue(ptr: UnsafeMutableRawPointer?) {
Expand Down