Skip to content
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.

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

Improper locking bugs due to the unrelesed locks before destorying #232

Open
ycaibb opened this issue Sep 7, 2021 · 1 comment
Open

Comments

@ycaibb
Copy link

ycaibb commented Sep 7, 2021

Hi, developers, thank you for your checking! Should the lock be released before destroying it(Line 158 and 164)? According to this, it says It shall be safe to destroy an initialized mutex that is unlocked.

neko/vm/threads.c

Lines 150 to 165 in 1df580c

pthread_attr_t attr;
pthread_attr_init(&attr);
pthread_attr_setdetachstate(&attr,PTHREAD_CREATE_DETACHED);
pthread_mutex_init(&p.lock,NULL);
pthread_mutex_lock(&p.lock);
// force the use of a the GC method to capture created threads
// this function should be defined in gc/gc.h
if( GC_pthread_create((pthread_t*)handle,&attr,&ThreadMain,&p) != 0 ) {
pthread_attr_destroy(&attr);
pthread_mutex_destroy(&p.lock);
return 0;
}
pthread_mutex_lock(&p.lock);
pthread_attr_destroy(&attr);
pthread_mutex_destroy(&p.lock);
return 1;

@ycaibb
Copy link
Author

ycaibb commented Sep 7, 2021

Also, it seems the lock(Line 154) would be released by the thread(Line 117), is my understanding correct? If so, I think the attribute of mutex should be set to pthread_mutexattr_setpshared to enable this:

The process-shared attribute is set to PTHREAD_PROCESS_SHARED to permit a mutex to be operated upon by any thread that has access to the memory where the mutex is allocated, even if the mutex is allocated in memory that is shared by multiple processes.

neko/vm/threads.c

Lines 110 to 124 in 1df580c

static THREAD_FUN ThreadMain( void *_p ) {
tparams *lp = (tparams*)_p;
tparams p = *lp;
p.init(p.param);
// we have the 'param' value on this thread C stack
// so it's safe to give back control to main thread
# ifdef NEKO_WINDOWS
ReleaseSemaphore(p.lock,1,NULL);
# else
pthread_mutex_unlock(&lp->lock);
# endif
clean_c_stack(40,clean_c_stack);
p.main(p.param);
return 0;
}

Thank you for your checking! Looking forward to the discussion.

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

No branches or pull requests

1 participant