On 01/30/2017 03:24 PM, Nadav Har'El wrote:
On Mon, Jan 30, 2017 at 3:26 PM, Justin Cinkelj
<justin.cink...@xlab.si <mailto:justin.cink...@xlab.si>> wrote:
On 01/29/2017 10:03 AM, Nadav Har'El wrote:
On Fri, Jan 27, 2017 at 12:26 AM, Justin Cinkelj
<justin.cink...@xlab.si <mailto:justin.cink...@xlab.si>> wrote:
The thread ID of thread running new app main() function is
returned.
I think this has the same bug we already solved in osv_execve() -
when application::run returns, if I understand correctly, the
thread may not be actually running, so its id is not yet set.
https://github.com/cloudius-systems/osv/blob/master/core/sched.cc#L911
<https://github.com/cloudius-systems/osv/blob/master/core/sched.cc#L911>
thread::thread(std::function<void ()> func, attr attr, bool main,
bool app)
That one should set the thread ID.
Unless I missed something?
(I'm interested into answer; likely I will use those 4 patches at
least temporarily, until we get final solution).
You are right, I was wrong - osv_execve() had a different problem in
that code running in the new thread set its own id.
We could have done something like you did now - get the thread's id
before it actually begins to run.
So what you did should work.
By the way, osv::run_background() instead of filling a thread
parameter, could return app (instead of just ignoring it), and the
caller could call the get_main_thread() method. Previously this wasn't
possible (most of the stuff in app.h was #ifdef _KERNEL) but I'm
trying to fix that now.
That will be nice. I should update my patches after you commit your
#ifdef _KERNEL modifications.
I guess we can solve this like we did for osv_execve() (i.e.,
creating a new thread first, and then running the new application
in the current thread) but I rather not go into this mess again :-(
Then we should not duplicate solution from osv_execve(), but just
call it. E.g replace call to application::run() with osv_execve().
That should work, the osv_execve only should not create new ELF
namespace.
How about the idea I raised in another email to keep a list of
osv::application pointers - instead of "thread ids" - in the
httpserver (httpserver can assign numeric ids to those if it wishes)?
This is bad in sense that yet another app_registry is added. I
just noticed there is already such thing in
https://github.com/cloudius-systems/osv/blob/master/core/osv_execve.cc#L11
<https://github.com/cloudius-systems/osv/blob/master/core/osv_execve.cc#L11>
It is there just to allow retrieval of exit code of apps - but
sure, it works only for those started via osv_execve() :/
Now we would add similar mechanism, but it would be specific for
apps started via httpserver.
It would be better to implement one mechanism, which would work
for all apps. Not something for httpserver only.
You're right. Something similar bothered me with your patch, which is
that it adds a new notion of completion of applications
(wait until the main thread returns) and tracking that.
SIgh, if question is "is it terminated?", then answer starts with
"please define terminated first" :/
I was interested if app is able to do any user-visible work (and naively
ignored fact there might be additional (detached) threads started by
that app).
If application object and thread stack are still allocated - they can't
do any computing without threads.
Also, unfortunately your patches do not solve the memory leak issue I
was trying to solve with the zombie applications
(I guess, though, that this is only important for artificial runs, not
what you're trying to do).
I didn't want to solve that one (1. you were already working on it, 2.
smaller patches are better)
I need to think about this some more :-(
Nadav.
--
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.
