On Thu, Oct 27, 2016 at 3:06 PM, Benoît Canet < [email protected]> wrote:
> We want the init of the elf object to use > the tls defined when the new thread is started. > > Delay this initialization. > > Signed-off-by: Benoît Canet <[email protected]> > --- > core/app.cc | 4 +++- > core/elf.cc | 49 ++++++++++++++++++++++++++++++++++++++++++++----- > include/osv/elf.hh | 6 +++++- > 3 files changed, 52 insertions(+), 7 deletions(-) > > diff --git a/core/app.cc b/core/app.cc > index 773d80e..0bfcedd 100644 > --- a/core/app.cc > +++ b/core/app.cc > @@ -174,7 +174,8 @@ application::application(const std::string& command, > > merge_in_environ(new_program, env); > prepare_argv(); > - _lib = current_program->get_library(_command); > + std::vector<std::string> extra_path; > + _lib = current_program->get_library(_command, extra_path, true); > } catch(const std::exception &e) { > throw launch_error(e.what()); > } > @@ -278,6 +279,7 @@ void application::main() > { > __libc_stack_end = __builtin_frame_address(0); > > + elf::get_program()->init_library(); > sched::thread::current()->set_name(_command); > > if (_main) { > diff --git a/core/elf.cc b/core/elf.cc > index efa574e..bf2137c 100644 > --- a/core/elf.cc > +++ b/core/elf.cc > @@ -1164,25 +1164,64 @@ program::load_object(std::string name, > std::vector<std::string> extra_path, > } > > std::shared_ptr<object> > -program::get_library(std::string name, std::vector<std::string> > extra_path) > +program::get_library(std::string name, std::vector<std::string> > extra_path, bool no_init) > { > SCOPE_LOCK(_mutex); > std::vector<std::shared_ptr<object>> loaded_objects; > auto ret = load_object(name, extra_path, loaded_objects); > + > + // Push the loaded object on a stack > + // init_library is to be called later at an arbitraty time > + // and operate on the list of loaded_objects. Since a library > + // can load another one like java.so does in OSv we want a stack > + // structure so each init_library call get it's corresponding > + // list of objects to operate on. > + > + // Build a vector of weak pointer so it will not prevent anyone > + // from unloading a .so properly while giving init_library some > + // safety. > + std::vector<std::weak_ptr<object>> weak_objects; > + for (auto obj: loaded_objects) { > + weak_objects.push_back(obj); > + } > + // push the weak pointer on the stack > + _loaded_objects_stack.push(weak_objects); > + > if (ret) { > ret->init_static_tls(); > } > + > + if (no_init) { > + return ret; > + } > + > + init_library(); > + > + return ret; > +} > + > +void program::init_library() > +{ > I just realized something... This function has no parameters? So you are not allowed to run multiple get_library() in parallel? Doesn't sound like a necessary restriction... > + // get the list of weak pointers before iterating on them > + std::vector<std::weak_ptr<object>> weak_objects = > + _loaded_objects_stack.top(); > + > // After loading the object and all its needed objects, run these > objects' > // init functions in reverse order (so those of deepest needed object > runs > // first) and finally make the loaded objects visible in search order. > - auto size = loaded_objects.size(); > + auto size = weak_objects.size(); > If we go ahead with allowing get_libary(no_init=true) on one thread and then init_library() later in a new thread (which is what you're proposing), we have a problem here: While we loaded all the objects in get_library(), we marked them with with setprivate(true) meaning only the current - at that time - thread can see them. When in init_library() here we want to run the initialization functions, we need to make the objects loaded visible to the thread which is current *now*. So in this point in the patch we need the following loop: for (unsigned i = 0; i < size; i++) { if (auto obj = weak_objects[i].lock()) { obj->setprivate(true); } } The setprivate(true) calls resets the identity of the only thread which can resolve symbols of from these objects to be *this* thread. > for (int i = size - 1; i >= 0; i--) { > - loaded_objects[i]->run_init_funcs(); > + if (auto obj = weak_objects[i].lock()) { > + obj->run_init_funcs(); > + } > } > for (unsigned i = 0; i < size; i++) { > - loaded_objects[i]->setprivate(false); > + if (auto obj = weak_objects[i].lock()) { > + obj->setprivate(false); > + } > } > - return ret; > + > + _loaded_objects_stack.pop(); > } > > void program::remove_object(object *ef) > diff --git a/include/osv/elf.hh b/include/osv/elf.hh > index fabc6e6..d26b22c 100644 > --- a/include/osv/elf.hh > +++ b/include/osv/elf.hh > @@ -11,6 +11,7 @@ > #include "fs/fs.hh" > #include <vector> > #include <map> > +#include <stack> > #include <memory> > #include <unordered_set> > #include <osv/types.h> > @@ -525,7 +526,9 @@ public: > * set_search_path(). > */ > std::shared_ptr<elf::object> > - get_library(std::string lib, std::vector<std::string> extra_path = > {}); > + get_library(std::string lib, std::vector<std::string> extra_path = > {}, bool no_init = false); > + > + void init_library(); > > /** > * Set the default search path for get_library(). > @@ -596,6 +599,7 @@ private: > > friend elf::file::~file(); > friend class object; > + std::stack<std::vector<std::weak_ptr<object>>> _loaded_objects_stack; > }; > > void create_main_program(); > -- > 2.7.4 > > -- > 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 [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
