This is a really cool idea. It will indeed work for alarms, because the
application-specific alarm function will call the application-specific
kill() function which will run the application's choice of handler.

However, I think it will not really work in places where unduplicated OSv
code calls kill() or related functions. For example:

1. In drivers/line-discipline.cc, we call kill(0, SIGINT). This will call
the main kill and the main handler - not the application's, so the
application's attempt to catch SIGINT will simply not work, even if there's
just one application (if started with the signal virtualization flag).
Arguably this is not an important problem - an application cannot hope to
catch SIGINT in this case, so it's ok that it doesn't.

2. When we have a page fault, division by zero, and similar exceptions,
code in exceptions.cc and core/mmu.cc calls the functions generate_signal()
and handle_mmap_fault() which are (to make a long story short) specialized
versions of kill. Although both these functions are in signal.cc and are
duplicated for each application, it doesn't help because the OSv kernel
calls them, so it uses the version in the OSv kernel, not the one in the
application. This might be a problem: If Java, for example, needs to
capture SIGSEGV to do something (not just for error reporting but for
actual correct operation), then its signal handler will be ignored. Did you
test Java with this patch?

One way we can solve this problem is perhaps for the generate_signal()
function to start by looking for a function by this name in the current
application (the one the current thread belongs to) and if we find one
which is not the one currently running, we run the one from the application
instead. This will obviously slow down this function, but I hope it's not
very frequently used (although it's worth checking how often some Java
program runs it, we can easily add a tracepoint if we don't already have
one, and count it with scripts/freq.py).

Maybe there are other ways to solve this. Or maybe we should just start by
having this feature only "partially" work as it does now, for only some
applications?

.


--
Nadav Har'El
n...@scylladb.com

On Wed, Mar 8, 2017 at 9:22 AM, Justin Cinkelj <justin.cink...@xlab.si>
wrote:

> Before patch, netperf netserver.so and netperf.so could not be run in
> the same VM. The netserver reports "error starting alarm timer,
> ret 3 errno 28".
>
> The patch make alarm (and also signals) per namespace specific, in the
> wat as environ was made per namespace specific in
> e93af8d3622ba1cf491ba12c780f05bb447de8ac and
> 1d3645b6c1a119ea34e1a3dca6ae814d4902905f.
>
> Signed-off-by: Justin Cinkelj <justin.cink...@xlab.si>
> ---
>  Makefile           |  9 ++++++++-
>  core/app.cc        | 10 ++++++++++
>  include/osv/app.hh |  2 ++
>  usr.manifest.skel  |  1 +
>  4 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 6846c5e..d982864 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -926,6 +926,7 @@ libc =
>  musl =
>  environ_libc =
>  environ_musl =
> +signal_libc =
>
>  ifeq ($(arch),x64)
>  musl_arch = x86_64
> @@ -995,6 +996,8 @@ environ_musl += env/putenv.c
>  environ_musl += env/setenv.c
>  environ_musl += env/unsetenv.c
>
> +signal_libc += signal.cc
> +
>  musl += ctype/__ctype_b_loc.o
>
>  musl += errno/strerror.o
> @@ -1830,10 +1833,14 @@ $(out)/bsd/%.o: COMMON += -DSMP
> -D'__FBSDID(__str__)=extern int __bogus__'
>
>  environ_sources = $(addprefix libc/, $(environ_libc))
>  environ_sources += $(addprefix musl/src/, $(environ_musl))
> +signal_sources = $(addprefix libc/, $(signal_libc))
>
>  $(out)/libenviron.so: $(environ_sources)
>         $(makedir)
>          $(call quiet, $(CC) $(CFLAGS) -shared -o $(out)/libenviron.so
> $(environ_sources), CC libenviron.so)
> +$(out)/libsignal.so: $(signal_sources)
> +       $(makedir)
> +        $(call quiet, $(CXX) $(CXXFLAGS) -shared -o $(out)/libsignal.so
> $(signal_sources), CXX libsignal.so)
>
>  bootfs_manifest ?= bootfs.manifest.skel
>
> @@ -1847,7 +1854,7 @@ $(bootfs_manifest_dep): phony
>         fi
>
>  $(out)/bootfs.bin: scripts/mkbootfs.py $(bootfs_manifest)
> $(bootfs_manifest_dep) $(tools:%=$(out)/%) \
> -               $(out)/zpool.so $(out)/zfs.so $(out)/libenviron.so
> +               $(out)/zpool.so $(out)/zfs.so $(out)/libenviron.so
> $(out)/libsignal.so
>         $(call quiet, olddir=`pwd`; cd $(out);
> $$olddir/scripts/mkbootfs.py -o bootfs.bin -d bootfs.bin.d -m
> $$olddir/$(bootfs_manifest) \
>                 -D jdkbase=$(jdkbase) -D gccbase=$(gccbase) -D \
>                 glibcbase=$(glibcbase) -D miscbase=$(miscbase), MKBOOTFS
> $@)
> diff --git a/core/app.cc b/core/app.cc
> index d623a61..b800522 100644
> --- a/core/app.cc
> +++ b/core/app.cc
> @@ -161,6 +161,7 @@ application::application(const std::string& command,
>          if (new_program) {
>              this->new_program();
>              clone_osv_environ();
> +            clone_osv_signal();
>              current_program = _program.get();
>          } else {
>              // Do it in a separate branch because elf::get_program()
> would not
> @@ -449,6 +450,15 @@ elf::program *application::program() {
>  }
>
>
> +void application::clone_osv_signal()
> +{
> +    _libsignal = _program->get_library("libsignal.so");
> +    if (!_libsignal) {
> +        abort("could not load libsignal.so\n");
> +        return;
> +    }
> +}
> +
>  void application::clone_osv_environ()
>  {
>      _libenviron = _program->get_library("libenviron.so");
> diff --git a/include/osv/app.hh b/include/osv/app.hh
> index 83e017e..d301551 100644
> --- a/include/osv/app.hh
> +++ b/include/osv/app.hh
> @@ -185,6 +185,7 @@ public:
>      elf::program *program();
>  private:
>      void new_program();
> +    void clone_osv_signal();
>      void clone_osv_environ();
>      void set_environ(const std::string &key, const std::string &value,
>                       bool new_program);
> @@ -213,6 +214,7 @@ private:
>      mutex _termination_mutex;
>      std::shared_ptr<elf::object> _lib;
>      std::shared_ptr<elf::object> _libenviron;
> +    std::shared_ptr<elf::object> _libsignal;
>      main_func_t* _main;
>      void (*_entry_point)();
>      static app_registry apps;
> diff --git a/usr.manifest.skel b/usr.manifest.skel
> index 583bdfe..73f58c1 100644
> --- a/usr.manifest.skel
> +++ b/usr.manifest.skel
> @@ -1,5 +1,6 @@
>  [manifest]
>  /libenviron.so: libenviron.so
> +/libsignal.so: libsignal.so
>  /zpool.so: zpool.so
>  /libzfs.so: libzfs.so
>  /libuutil.so: libuutil.so
> --
> 2.9.3
>
> --
> 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.
>

-- 
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.

Reply via email to