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

pthread threads are not being created in correct "process" context #15

Closed
mato opened this issue May 13, 2015 · 8 comments
Closed

pthread threads are not being created in correct "process" context #15

mato opened this issue May 13, 2015 · 8 comments
Assignees
Labels

Comments

@mato
Copy link
Member

mato commented May 13, 2015

After recent changes to pthreads implementation, threads created by pthread_create() are not being created in the correct "process". This results in eg. file descriptors passed from the main thread to a pthread not being visible and returning EBADF.

@mato mato added the bug label May 13, 2015
@mato mato self-assigned this May 13, 2015
mato added a commit that referenced this issue May 13, 2015
@DerangedMonkeyNinja
Copy link
Contributor

I think this is due to the main thread getting run as PID 3, but any threads spawned by the main thread end using the default LWP context, and thus think they are PID 1.

At least, I was able to work around a similar issue by calling rump_pub_lwproc_newlwp(1) in main before doing anything with descriptors. I wasn't sure if this was really a bug or not though, due to the rump man page saying that "the association between host threads and the rump kernel curlwp is left to the caller"...

@anttikantee
Copy link
Member

It's a bug in _lwp.c, makelwp() should create threads in the same rump kernel process as the caller.

Instead of you calling rump_pub_lwproc_newlwp(1) in main() we should be calling rump_pub_lwproc_newlwp(getpid()) and switching to that lwp in the newly created bmk_thread.

The main reason why rumprun() calls newproc is so that you can have processes running with UID != 0. Also, later, the intention is supporting multiple calls to rumprun() in the same rumprun instance, i.e. "multiprocess" support.

mato added a commit that referenced this issue May 14, 2015
Create a rump kernel lwp for each pthread lwp which is created from
an existing (non-implicit) rump kernel process.

WIP for resolving issue #15
@mato
Copy link
Member Author

mato commented May 14, 2015

I've pushed a work in progress patch with some debugging printfs to the wip-lwp branch at 026bbb6.

However: it is not behaving as expected. I call rump_pub_lwproc_switch(next->rl_klwp) in schedhook, but rump_pub_lwproc_curlwp() inside the newly created thread is still returning 0x0.

Could there be some interaction going on with curlwp using __thread?

Example output from pthread_test with the patch:

mounted tmpfs on /tmp
makelwp: called in implicit context, not creating klwp
makelwp: called in implicit context, not creating klwp

=== calling "rumprun-xen" main() ===

testing pthread_join
makelwp: curlwp 0x596800 me->rl_klwp 0x596800
makelwp: new curlwp is 0x95f800
thread 0x0, curlwp 0x0
lwp_exit: me->rl_klwp 0x95f800 curlwp 0x0
success
makelwp: curlwp 0x95f800 me->rl_klwp 0x596800
makelwp: new curlwp is 0xa59800
panic: lwp 0x596800 (2:0) already running
rump kernel halting...
halted
PANIC: rumpuser panic
minios: halting, reason=0

@mato
Copy link
Member Author

mato commented May 14, 2015

N.B. The releaselwp() call in _lwp_exit() is commented out deliberately since it would cause an assertion failure due to being called with curlwp=0x0.

@mato
Copy link
Member Author

mato commented May 14, 2015

Ok, so sched_switch() calls schedhook and then sets the TLS to the thread we are switching to. This does not help if schedhook needs to call rump_pub_lwproc_switch().

I've change the order of the calls and added a few more fixes to call rump_pub_lwproc_releaselwp() from lwp_exit(). pthread_test now passes, including the file descriptor test.

mato added a commit that referenced this issue May 14, 2015
Create a rump kernel lwp for each pthread lwp which is created from
an existing (non-implicit) rump kernel process.

(See issue #15)
@mato
Copy link
Member Author

mato commented May 14, 2015

Force-pushed a new version, using a trampoline to set curlwp. Many thanks to @anttikantee for the jedi tips in figuring out how to implement this properly.

@anttikantee If you're happy with the patch as is at 9e5f4e5 then I think we can merge it. pthread_test passes and mysqld works :)

@anttikantee
Copy link
Member

Almost there. See comments in the patch.

thanks

mato added a commit that referenced this issue May 14, 2015
Create a rump kernel lwp for each pthread lwp which is created from
an existing (non-implicit) rump kernel process.

(See issue #15)
@mato
Copy link
Member Author

mato commented May 14, 2015

Comments addressed and everything works as expected. Merged.

@mato mato closed this as completed May 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants