Callback thread safety issue #486
Comments
|
The instances of |
|
@saudet ThreadPoolExecutor executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(threadCount);
List<Future> futures = new ArrayList<>();
NativeClass2 processor = new ...;
NativeClass1 ctxes = new NativeClass1 [threadCount];
for (int i = 0; i < threadCount; i++) {
ctxes[i] = ...;
}
for (int i = 0; i < threadCount; i++) {
futures.add(executor.submit(()->{
NativeClass1 ctx = ctxes[i];
for (Query q : queries) {
processor.process(ctx, q);
}
});
}
System.out.println(ctxes.length);I do not quite understand why |
|
|
|
I see the problem. |
|
I see huge lock contention with java callback when thread number is high. |
|
seems the lock contention is from here static JavaCPP_noinline bool JavaCPP_getEnv(JNIEnv** env) {
bool attached = false;
JavaVM *vm = JavaCPP_vm;
if (vm == NULL) {
JavaCPP_log("Could not get any created JavaVM.");
*env = NULL;
return false;
}
#if defined(__linux__) || defined(__APPLE__)
pthread_mutex_lock(&JavaCPP_lock);
pthread_once(&JavaCPP_once, JavaCPP_create_pthread_key);
if ((*env = (JNIEnv *)pthread_getspecific(JavaCPP_current_env)) != NULL) {
attached = true;
goto done;
}
#endif
unsigned char JavaCPP_com_mycompany_myproject2_presets_myproject_00024FilterFunction::operator()(uint64_t arg0) {
jboolean rarg = 0;
JNIEnv* env;
bool attached = JavaCPP_getEnv(&env);
if (env == NULL) {
goto end;
} |
|
That code was added by @tmm1 to have threads automatically detach themselves when they exit. |
|
Can we change the implementation to use In my scenario, thread exit with the program or a little earlier before the program. |
|
I see why pthread specific is used from #243 . static JavaCPP_noinline bool JavaCPP_getEnv(JNIEnv** env) {
bool attached = false;
JavaVM *vm = JavaCPP_vm;
if (vm == NULL) {
JavaCPP_log("Could not get any created JavaVM.");
*env = NULL;
return false;
}
#if defined(__linux__) || defined(__APPLE__)
pthread_mutex_lock(&JavaCPP_lock);
pthread_once(&JavaCPP_once, JavaCPP_create_pthread_key);
if ((*env = (JNIEnv *)pthread_getspecific(JavaCPP_current_env)) != NULL) {
attached = true;
goto done;
}
#endif
if (vm->GetEnv((void**)env, JNI_VERSION_1_6) != JNI_OK) {
struct {
JNIEnv **env;
operator JNIEnv**() { return env; } // Android JNI
operator void**() { return (void**)env; } // standard JNI
} env2 = { env };
JavaVMAttachArgs args;
args.version = JNI_VERSION_1_6;
args.group = NULL;
char name[64] = {0};
#ifdef _WIN32
sprintf(name, "JavaCPP Thread ID %lu", GetCurrentThreadId());
#elif defined(__APPLE__)
sprintf(name, "JavaCPP Thread ID %u", pthread_mach_thread_np(pthread_self()));
#else
sprintf(name, "JavaCPP Thread ID %lu", pthread_self());
#endif
args.name = name;
if (vm->AttachCurrentThread(env2, &args) != JNI_OK) {
JavaCPP_log("Could not attach the JavaVM to the current thread.");
*env = NULL;
goto done;
}
#if defined(__linux__) || defined(__APPLE__)
pthread_setspecific(JavaCPP_current_env, *env);
#endif
attached = true;
}
if (JavaCPP_vm == NULL) {
if (JNI_OnLoad(vm, NULL) < 0) {
JavaCPP_detach(attached);
*env = NULL;
goto done;
}
}
done:
#if defined(__linux__) || defined(__APPLE__)
pthread_mutex_unlock(&JavaCPP_lock);
#endif
return attached;
} |
We need pthreads anyway to detach threads on exit. I don't think C++ offers something like that, yet.
Ok, in that case then we can skip all that. I've made it possible to do that in commit e5495a2 by defining NO_JNI_DETACH_THREAD, for example, by annotating classes with
Could you remove the lock and see how it fares in your application? If it doesn't seem to cause problems, I'm fine with removing it. In any case, please open a pull request with what works for you. Thanks! |
|
Removing the lock make our benchmark 5 times faster. #489 is created. |
In our project, Java implemented callback is passed to native code. The function signature looks like
std::function<bool(uint64_t)>.In our testing program, a ThreadPool is created. Each thread runs the same logic, creating one callback and passing the callback to native code. Pseudo code looks like this
When there are 29 or less threads created, the testing program runs fine. However creating 30 or more threads will lead to crashing(core dump).
The generated jni code contains static callback array, which seems not to be protected by lock or synchronizes with proper memory order. I do not know if that's ok. (Besides, I do not see usage of rptr->ptr in
operator()(uint64_t arg0))The text was updated successfully, but these errors were encountered: