On 08/21/2016 04:38 PM, Nadav Har'El wrote:
On Sun, Aug 21, 2016 at 5:22 PM, Nadav Har'El <n...@scylladb.com
<mailto:n...@scylladb.com>> wrote:
Hi Justin,
Unless I'm misunderstanding something, I think there is a race in
your code: osv_execve() spawns a new thread to run a function
thread_run_app_in_namespace(), and this will eventually set the
value pointed by pid_t *thread_id. But that happens some time in
the future, and at the time that osv_execve() returns, the
thread_id might not already be set, and you can't know exactly
when it will be set. So how can the caller actually use this API?
Is he supposed to loop checking if the value is no longer zero and
sleep until it isn't? Is this what you intended?
I've been thinking about this some more, and why you had to change
your osv_execve() implementation - which already returned a thread id,
but the "wrong" one:
The "original sin", so to speak, is that both osv_execve() and
application::start() which it calls (through osv::run()), start a new
thread. So if we have thread A and run osv_execve(), it creates thread
B, and in it creates thread C to run the actual application code.
osv_execve() returned B, but you wanted it to create C.
But why must we have both B and C? Wouldn't it be better if we had
just one?
osv::run() currently does:
auto app = osv::application::run(path, args, new_program, env);
app->join();
Yes, that's the main and only reason. The wrong tid was good enough to
later detect when newly started app terminated (osv_waittid() from
core/osv_execve.cc). But now I need to setaffinity of threads in the new
app. And the wrong thread is - the wrong one...
What if instead it called a new int osv::application::run_and_join(...)?
This run_and_join() will call a new start_and_join(), which unlike
start() will not create a new thread to run the code - but rather run
it in the thread it was called in. The start_and_join() will need to
also set the current_app field on the existing thread correctly (and
revert it when the main function returns).
If we do this, osv_execve's returned pid will be the correct one, and
won't need to change.
Does this idea make sense to you?
Sounds good to me. I guess best I just drop the current patch, and wait
on the new one.
BR Justin
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
