unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
						download: 
* [PATCH] Add worker_exec configuration option
@ 2017-03-08 18:44 Jeremy Evans
  2017-03-08 20:02 ` Eric Wong
  0 siblings, 1 reply; 12+ results
From: Jeremy Evans @ 2017-03-08 18:44 UTC (permalink / raw)
  To: unicorn-public

The worker_exec configuration option makes all worker processes
exec after forking.  This initializes the worker processes with
separate memory layouts, defeating address space discovery
attacks on operating systems supporting address space layout
randomization, such as Linux, MacOS X, NetBSD, OpenBSD, and
Solaris.

Support for execing workers is very similar to support for reexecing
the master process.  The main difference is the worker's to_i and
master pipes also need to be inherited after worker exec just as the
listening sockets need to be inherited after reexec.
---
 lib/unicorn/configurator.rb | 10 ++++++++
 lib/unicorn/http_server.rb  | 56 ++++++++++++++++++++++++++++++++++++++++++---
 lib/unicorn/worker.rb       |  5 ++--
 3 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index 7ed5ffa..f69f220 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -53,6 +53,7 @@ class Unicorn::Configurator
         server.logger.info("worker=#{worker.nr} ready")
       },
     :pid => nil,
+    :worker_exec => false,
     :preload_app => false,
     :check_client_connection => false,
     :rewindable_input => true, # for Rack 2.x: (Rack::VERSION[0] <= 1),
@@ -239,6 +240,15 @@ def timeout(seconds)
     set[:timeout] = seconds > max ? max : seconds
   end
 
+  # Whether to exec in each worker process after forking.  This changes the
+  # memory layout of each worker process, which is a security feature designed
+  # to defeat possible address space discovery attacks.  Note that using
+  # worker_exec only makes sense if you are not preloading the application,
+  # and will result in higher memory usage.
+  def worker_exec(bool)
+    set_bool(:worker_exec, bool)
+  end
+
   # sets the current number of worker_processes to +nr+.  Each worker
   # process will serve exactly one client at a time.  You can
   # increment or decrement this value at runtime by sending SIGTTIN
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index ef897ad..d68d7d8 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -15,7 +15,7 @@ class Unicorn::HttpServer
                 :before_fork, :after_fork, :before_exec,
                 :listener_opts, :preload_app,
                 :orig_app, :config, :ready_pipe, :user
-  attr_writer   :after_worker_exit, :after_worker_ready
+  attr_writer   :after_worker_exit, :after_worker_ready, :worker_exec
 
   attr_reader :pid, :logger
   include Unicorn::SocketHelper
@@ -105,6 +105,14 @@ def initialize(app, options = {})
     # list of signals we care about and trap in master.
     @queue_sigs = [
       :WINCH, :QUIT, :INT, :TERM, :USR1, :USR2, :HUP, :TTIN, :TTOU ]
+
+    if worker_nr = ENV['UNICORN_WORKER_NR']
+      @worker_nr = worker_nr.to_i
+      @master_pid = Process.ppid
+    else
+      @master_pid = $$
+    end
+
   end
 
   # Runs the thing.  Returns self so you can run join on it
@@ -113,7 +121,6 @@ def start
     # this pipe is used to wake us up from select(2) in #join when signals
     # are trapped.  See trap_deferred.
     @self_pipe.replace(Unicorn.pipe)
-    @master_pid = $$
 
     # setup signal handlers before writing pid file in case people get
     # trigger happy and send signals as soon as the pid file exists.
@@ -459,6 +466,39 @@ def reexec
     proc_name 'master (old)'
   end
 
+  def worker_exec(worker)
+    listener_fds = {}
+    LISTENERS.each do |sock|
+      sock.close_on_exec = false
+      listener_fds[sock.fileno] = sock
+    end
+    ENV['UNICORN_FD'] = listener_fds.keys.join(',')
+    ENV['UNICORN_WORKER_NR'] = worker.nr.to_s
+    ENV['UNICORN_WORKER_TO_IO'] = worker.to_io.fileno.to_s
+    ENV['UNICORN_WORKER_MASTER'] = worker.master.fileno.to_s
+    Dir.chdir(START_CTX[:cwd])
+    cmd = [ START_CTX[0] ].concat(START_CTX[:argv])
+
+    listener_fds[worker.to_io.fileno] = worker.to_io
+    listener_fds[worker.master.fileno] = worker.master
+
+    # avoid leaking FDs we don't know about, but let before_exec
+    # unset FD_CLOEXEC, if anything else in the app eventually
+    # relies on FD inheritence.
+    (3..1024).each do |io|
+      next if listener_fds.include?(io)
+      io = IO.for_fd(io) rescue next
+      io.autoclose = false
+      io.close_on_exec = true
+    end
+
+    # exec(command, hash) works in at least 1.9.1+, but will only be
+    # required in 1.9.4/2.0.0 at earliest.
+    cmd << listener_fds
+    logger.info "executing worker #{cmd.inspect} (in #{Dir.pwd})"
+    exec(*cmd)
+  end
+
   # forcibly terminate all workers that haven't checked in in timeout seconds.  The timeout is implemented using an unlinked File
   def murder_lazy_workers
     next_sleep = @timeout - 1
@@ -495,6 +535,12 @@ def after_fork_internal
   end
 
   def spawn_missing_workers
+    if @worker_nr
+      worker = Unicorn::Worker.new(@worker_nr, [ENV["UNICORN_WORKER_TO_IO"], ENV["UNICORN_WORKER_MASTER"]].map{|i| Kgio::Pipe.for_fd(i.to_i)})
+      worker_loop(worker)
+      exit
+    end
+
     worker_nr = -1
     until (worker_nr += 1) == @worker_processes
       @workers.value?(worker_nr) and next
@@ -505,7 +551,11 @@ def spawn_missing_workers
         worker.atfork_parent
       else
         after_fork_internal
-        worker_loop(worker)
+        if @worker_exec
+          worker_exec(worker)
+        else
+          worker_loop(worker)
+        end
         exit
       end
     end
diff --git a/lib/unicorn/worker.rb b/lib/unicorn/worker.rb
index e22c1bf..8bbac5e 100644
--- a/lib/unicorn/worker.rb
+++ b/lib/unicorn/worker.rb
@@ -12,18 +12,19 @@ class Unicorn::Worker
   # :stopdoc:
   attr_accessor :nr, :switched
   attr_reader :to_io # IO.select-compatible
+  attr_reader :master
 
   PER_DROP = Raindrops::PAGE_SIZE / Raindrops::SIZE
   DROPS = []
 
-  def initialize(nr)
+  def initialize(nr, pipe=nil)
     drop_index = nr / PER_DROP
     @raindrop = DROPS[drop_index] ||= Raindrops.new(PER_DROP)
     @offset = nr % PER_DROP
     @raindrop[@offset] = 0
     @nr = nr
     @switched = false
-    @to_io, @master = Unicorn.pipe
+    @to_io, @master = pipe || Unicorn.pipe
   end
 
   def atfork_child # :nodoc:
-- 
2.11.0


^ permalink raw reply	[relevance 7%]

* Re: [PATCH] Add worker_exec configuration option
  2017-03-08 18:44 [PATCH] Add worker_exec configuration option Jeremy Evans
@ 2017-03-08 20:02 ` Eric Wong
  2017-03-09  4:52   ` Jeremy Evans
  2017-03-09 19:41   ` [PATCH] Add worker_exec configuration option V2 Jeremy Evans
  0 siblings, 2 replies; 12+ results
From: Eric Wong @ 2017-03-08 20:02 UTC (permalink / raw)
  To: Jeremy Evans; +Cc: unicorn-public

Jeremy Evans <code@jeremyevans.net> wrote:
> The worker_exec configuration option makes all worker processes
> exec after forking.  This initializes the worker processes with
> separate memory layouts, defeating address space discovery
> attacks on operating systems supporting address space layout
> randomization, such as Linux, MacOS X, NetBSD, OpenBSD, and
> Solaris.
> 
> Support for execing workers is very similar to support for reexecing
> the master process.  The main difference is the worker's to_i and
> master pipes also need to be inherited after worker exec just as the
> listening sockets need to be inherited after reexec.

Thanks, this seems like an acceptable feature.
Some comments below....

> +++ b/lib/unicorn/http_server.rb
> @@ -105,6 +105,14 @@ def initialize(app, options = {})
>      # list of signals we care about and trap in master.
>      @queue_sigs = [
>        :WINCH, :QUIT, :INT, :TERM, :USR1, :USR2, :HUP, :TTIN, :TTOU ]
> +
> +    if worker_nr = ENV['UNICORN_WORKER_NR']
> +      @worker_nr = worker_nr.to_i
> +      @master_pid = Process.ppid
> +    else
> +      @master_pid = $$
> +    end
> +
>    end

We should nil @worker_nr in the normal case to avoid introducing
new warnings on uninitialized ivars.

>    # Runs the thing.  Returns self so you can run join on it
> @@ -113,7 +121,6 @@ def start
>      # this pipe is used to wake us up from select(2) in #join when signals
>      # are trapped.  See trap_deferred.
>      @self_pipe.replace(Unicorn.pipe)
> -    @master_pid = $$

Any particular reason setting @master_pid was moved to initialize?

Maybe we (or anybody else) has tests or other code which do:

	  foo = Unicorn::HttpServer.new ...
	  pid = fork do
	    foo.start.join
	  end

Which would be broken by the movement from start => initialize

Again, I don't consider this a stable API and don't want people
depending on our internals; but I will also avoid breaking
existing behavior whenever possible.

> @@ -459,6 +466,39 @@ def reexec
>      proc_name 'master (old)'
>    end
>  
> +  def worker_exec(worker)
> +    listener_fds = {}
> +    LISTENERS.each do |sock|
> +      sock.close_on_exec = false
> +      listener_fds[sock.fileno] = sock
> +    end
> +    ENV['UNICORN_FD'] = listener_fds.keys.join(',')
> +    ENV['UNICORN_WORKER_NR'] = worker.nr.to_s

> +    ENV['UNICORN_WORKER_TO_IO'] = worker.to_io.fileno.to_s
> +    ENV['UNICORN_WORKER_MASTER'] = worker.master.fileno.to_s

We can cram these two FDs into one UNICORN_WORKER_FD
comma-delimited variable like UNICORN_FD.  The underlying
getenv/setenv/putenv functions are all O(n), so keeping env
smaller would be nice.

And we can even avoid modifying ENV on Ruby 1.9+ by
using exec(env, cmd, options...) or even Process.spawn (see below)

> +    Dir.chdir(START_CTX[:cwd])
> +    cmd = [ START_CTX[0] ].concat(START_CTX[:argv])
> +
> +    listener_fds[worker.to_io.fileno] = worker.to_io
> +    listener_fds[worker.master.fileno] = worker.master
> +
> +    # avoid leaking FDs we don't know about, but let before_exec
> +    # unset FD_CLOEXEC, if anything else in the app eventually
> +    # relies on FD inheritence.

Comment seems blindly copy+pasted, since before_exec does not
seem to exist, for this...

> +    (3..1024).each do |io|
> +      next if listener_fds.include?(io)
> +      io = IO.for_fd(io) rescue next
> +      io.autoclose = false
> +      io.close_on_exec = true
> +    end

I suppose we should probably use Process.getrlimit, here,
maybe skip the loop entirely for Ruby 2.0.0+ (which made
close-on-exec the default).

This will be a separate patch...


Since there's no before_exec hook as with SIGUSR2, I think we
can do all of this setup in master and use Process.spawn instead
of fork+exec explicitly.  Process.spawn transparently allows the use
of vfork+exec to avoid fork overhead on some systems.

> +    # exec(command, hash) works in at least 1.9.1+, but will only be
> +    # required in 1.9.4/2.0.0 at earliest.

I guess it's time to update that comment, since 1.9.4 never
happened :)

> +    cmd << listener_fds
> +    logger.info "executing worker #{cmd.inspect} (in #{Dir.pwd})"
> +    exec(*cmd)
> +  end
> +
>    # forcibly terminate all workers that haven't checked in in timeout seconds.  The timeout is implemented using an unlinked File
>    def murder_lazy_workers
>      next_sleep = @timeout - 1
> @@ -495,6 +535,12 @@ def after_fork_internal
>    end
>  
>    def spawn_missing_workers
> +    if @worker_nr
> +      worker = Unicorn::Worker.new(@worker_nr, [ENV["UNICORN_WORKER_TO_IO"], ENV["UNICORN_WORKER_MASTER"]].map{|i| Kgio::Pipe.for_fd(i.to_i)})

Please keep new code wrapped to 80 chars or less.  I'm still
using hardware that predates this project and my eyes will
only get worse with age.

> @@ -505,7 +551,11 @@ def spawn_missing_workers
>          worker.atfork_parent
>        else
>          after_fork_internal
> -        worker_loop(worker)
> +        if @worker_exec
> +          worker_exec(worker)
> +        else
> +          worker_loop(worker)
> +        end

I prefer the ternary operator for simple cases like this.  It is
more compact in terms of screen space and VM bytecode.  But this
may not be changed at all if we switch to using Process.spawn,
instead.

The rest looks fine.  Thanks.

^ permalink raw reply	[relevance 10%]

* Re: [PATCH] Add worker_exec configuration option
  2017-03-08 20:02 ` Eric Wong
@ 2017-03-09  4:52   ` Jeremy Evans
  2017-03-09 13:57     ` Eric Wong
  2017-03-09 19:41   ` [PATCH] Add worker_exec configuration option V2 Jeremy Evans
  1 sibling, 1 reply; 12+ results
From: Jeremy Evans @ 2017-03-09  4:52 UTC (permalink / raw)
  To: Eric Wong; +Cc: unicorn-public

On 03/08 08:02, Eric Wong wrote:
> Jeremy Evans <code@jeremyevans.net> wrote:
> > The worker_exec configuration option makes all worker processes
> > exec after forking.  This initializes the worker processes with
> > separate memory layouts, defeating address space discovery
> > attacks on operating systems supporting address space layout
> > randomization, such as Linux, MacOS X, NetBSD, OpenBSD, and
> > Solaris.
> > 
> > Support for execing workers is very similar to support for reexecing
> > the master process.  The main difference is the worker's to_i and
> > master pipes also need to be inherited after worker exec just as the
> > listening sockets need to be inherited after reexec.
> 
> Thanks, this seems like an acceptable feature.

<snip>

Eric,

Thanks for your detailed review.  I will work on an updated patch and
try to send it tomorrow or Friday.

Thanks,
Jeremy

^ permalink raw reply	[relevance 14%]

* Re: [PATCH] Add worker_exec configuration option
  2017-03-09  4:52   ` Jeremy Evans
@ 2017-03-09 13:57     ` Eric Wong
  0 siblings, 0 replies; 12+ results
From: Eric Wong @ 2017-03-09 13:57 UTC (permalink / raw)
  To: Jeremy Evans; +Cc: unicorn-public

Jeremy Evans <code@jeremyevans.net> wrote:
> Thanks for your detailed review.  I will work on an updated patch and
> try to send it tomorrow or Friday.

No problem and no rush; I might be out a bit the next few days.

^ permalink raw reply	[relevance 14%]

* Re: [PATCH] Add worker_exec configuration option V2
  2017-03-08 20:02 ` Eric Wong
  2017-03-09  4:52   ` Jeremy Evans
@ 2017-03-09 19:41   ` Jeremy Evans
  2017-03-10 21:19     ` Eric Wong
  1 sibling, 1 reply; 12+ results
From: Jeremy Evans @ 2017-03-09 19:41 UTC (permalink / raw)
  To: Eric Wong; +Cc: unicorn-public

On 03/08 08:02, Eric Wong wrote:
> Jeremy Evans <code@jeremyevans.net> wrote:
> > The worker_exec configuration option makes all worker processes
> > exec after forking.  This initializes the worker processes with
> > separate memory layouts, defeating address space discovery
> > attacks on operating systems supporting address space layout
> > randomization, such as Linux, MacOS X, NetBSD, OpenBSD, and
> > Solaris.
> > 
> > Support for execing workers is very similar to support for reexecing
> > the master process.  The main difference is the worker's to_i and
> > master pipes also need to be inherited after worker exec just as the
> > listening sockets need to be inherited after reexec.
> 
> Thanks, this seems like an acceptable feature.

Eric,

Here's V2 of the patch, which I think should address all of the issues
you pointed out.

Thanks,
Jeremy

From 8e68bf8c6a8b91704f31dd9b9a62e6f2e330e380 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Wed, 8 Mar 2017 10:19:02 -0800
Subject: [PATCH] Add worker_exec configuration option

The worker_exec configuration option makes all worker processes
exec after forking.  This initializes the worker processes with
separate memory layouts, defeating address space discovery
attacks on operating systems supporting address space layout
randomization, such as Linux, MacOS X, NetBSD, OpenBSD, and
Solaris.

Support for execing workers is very similar to support for reexecing
the master process.  The main difference is the worker's to_i and
master pipes also need to be inherited after worker exec just as the
listening sockets need to be inherited after reexec.

Because execing working is similar to reexecing the master, this
extracts a couple of methods from reexec (listener_sockets and
close_sockets_on_exec), so they can be reused in worker_spawn.
---
 lib/unicorn/configurator.rb | 10 ++++++
 lib/unicorn/http_server.rb  | 83 ++++++++++++++++++++++++++++++++++-----------
 lib/unicorn/worker.rb       |  5 +--
 3 files changed, 77 insertions(+), 21 deletions(-)

diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index 7ed5ffa..f69f220 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -53,6 +53,7 @@ class Unicorn::Configurator
         server.logger.info("worker=#{worker.nr} ready")
       },
     :pid => nil,
+    :worker_exec => false,
     :preload_app => false,
     :check_client_connection => false,
     :rewindable_input => true, # for Rack 2.x: (Rack::VERSION[0] <= 1),
@@ -239,6 +240,15 @@ def timeout(seconds)
     set[:timeout] = seconds > max ? max : seconds
   end
 
+  # Whether to exec in each worker process after forking.  This changes the
+  # memory layout of each worker process, which is a security feature designed
+  # to defeat possible address space discovery attacks.  Note that using
+  # worker_exec only makes sense if you are not preloading the application,
+  # and will result in higher memory usage.
+  def worker_exec(bool)
+    set_bool(:worker_exec, bool)
+  end
+
   # sets the current number of worker_processes to +nr+.  Each worker
   # process will serve exactly one client at a time.  You can
   # increment or decrement this value at runtime by sending SIGTTIN
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index ef897ad..a5bd2c4 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -15,7 +15,7 @@ class Unicorn::HttpServer
                 :before_fork, :after_fork, :before_exec,
                 :listener_opts, :preload_app,
                 :orig_app, :config, :ready_pipe, :user
-  attr_writer   :after_worker_exit, :after_worker_ready
+  attr_writer   :after_worker_exit, :after_worker_ready, :worker_exec
 
   attr_reader :pid, :logger
   include Unicorn::SocketHelper
@@ -105,6 +105,14 @@ def initialize(app, options = {})
     # list of signals we care about and trap in master.
     @queue_sigs = [
       :WINCH, :QUIT, :INT, :TERM, :USR1, :USR2, :HUP, :TTIN, :TTOU ]
+
+    @worker_data = if worker_data = ENV['UNICORN_WORKER']
+      worker_data = worker_data.split(',').map!(&:to_i)
+      worker_data[1] = worker_data.slice!(1..2).map do |i|
+        Kgio::Pipe.for_fd(i)
+      end
+      worker_data
+    end
   end
 
   # Runs the thing.  Returns self so you can run join on it
@@ -113,7 +121,7 @@ def start
     # this pipe is used to wake us up from select(2) in #join when signals
     # are trapped.  See trap_deferred.
     @self_pipe.replace(Unicorn.pipe)
-    @master_pid = $$
+    @master_pid = @worker_data ? Process.ppid : $$
 
     # setup signal handlers before writing pid file in case people get
     # trigger happy and send signals as soon as the pid file exists.
@@ -430,11 +438,7 @@ def reexec
     end
 
     @reexec_pid = fork do
-      listener_fds = {}
-      LISTENERS.each do |sock|
-        sock.close_on_exec = false
-        listener_fds[sock.fileno] = sock
-      end
+      listener_fds = listener_sockets
       ENV['UNICORN_FD'] = listener_fds.keys.join(',')
       Dir.chdir(START_CTX[:cwd])
       cmd = [ START_CTX[0] ].concat(START_CTX[:argv])
@@ -442,12 +446,7 @@ def reexec
       # avoid leaking FDs we don't know about, but let before_exec
       # unset FD_CLOEXEC, if anything else in the app eventually
       # relies on FD inheritence.
-      (3..1024).each do |io|
-        next if listener_fds.include?(io)
-        io = IO.for_fd(io) rescue next
-        io.autoclose = false
-        io.close_on_exec = true
-      end
+      close_sockets_on_exec(listener_fds)
 
       # exec(command, hash) works in at least 1.9.1+, but will only be
       # required in 1.9.4/2.0.0 at earliest.
@@ -459,6 +458,40 @@ def reexec
     proc_name 'master (old)'
   end
 
+  def worker_spawn(worker)
+    listener_fds = listener_sockets
+    env = {}
+    env['UNICORN_FD'] = listener_fds.keys.join(',')
+
+    listener_fds[worker.to_io.fileno] = worker.to_io
+    listener_fds[worker.master.fileno] = worker.master
+
+    worker_info = [worker.nr, worker.to_io.fileno, worker.master.fileno]
+    env['UNICORN_WORKER'] = worker_info.join(',')
+
+    close_sockets_on_exec(listener_fds)
+
+    Process.spawn(env, START_CTX[0], *START_CTX[:argv], listener_fds)
+  end
+
+  def listener_sockets
+    listener_fds = {}
+    LISTENERS.each do |sock|
+      sock.close_on_exec = false
+      listener_fds[sock.fileno] = sock
+    end
+    listener_fds
+  end
+
+  def close_sockets_on_exec(sockets)
+    (3..1024).each do |io|
+      next if sockets.include?(io)
+      io = IO.for_fd(io) rescue next
+      io.autoclose = false
+      io.close_on_exec = true
+    end
+  end
+
   # forcibly terminate all workers that haven't checked in in timeout seconds.  The timeout is implemented using an unlinked File
   def murder_lazy_workers
     next_sleep = @timeout - 1
@@ -495,19 +528,31 @@ def after_fork_internal
   end
 
   def spawn_missing_workers
+    if @worker_data
+      worker = Unicorn::Worker.new(*@worker_data)
+      after_fork_internal
+      worker_loop(worker)
+      exit
+    end
+
     worker_nr = -1
     until (worker_nr += 1) == @worker_processes
       @workers.value?(worker_nr) and next
       worker = Unicorn::Worker.new(worker_nr)
       before_fork.call(self, worker)
-      if pid = fork
-        @workers[pid] = worker
-        worker.atfork_parent
+
+      pid = if @worker_exec
+        worker_spawn(worker)
       else
-        after_fork_internal
-        worker_loop(worker)
-        exit
+        fork do
+          after_fork_internal
+          worker_loop(worker)
+          exit
+        end
       end
+
+      @workers[pid] = worker
+      worker.atfork_parent
     end
     rescue => e
       @logger.error(e) rescue nil
diff --git a/lib/unicorn/worker.rb b/lib/unicorn/worker.rb
index e22c1bf..8bbac5e 100644
--- a/lib/unicorn/worker.rb
+++ b/lib/unicorn/worker.rb
@@ -12,18 +12,19 @@ class Unicorn::Worker
   # :stopdoc:
   attr_accessor :nr, :switched
   attr_reader :to_io # IO.select-compatible
+  attr_reader :master
 
   PER_DROP = Raindrops::PAGE_SIZE / Raindrops::SIZE
   DROPS = []
 
-  def initialize(nr)
+  def initialize(nr, pipe=nil)
     drop_index = nr / PER_DROP
     @raindrop = DROPS[drop_index] ||= Raindrops.new(PER_DROP)
     @offset = nr % PER_DROP
     @raindrop[@offset] = 0
     @nr = nr
     @switched = false
-    @to_io, @master = Unicorn.pipe
+    @to_io, @master = pipe || Unicorn.pipe
   end
 
   def atfork_child # :nodoc:
-- 
2.11.0


^ permalink raw reply	[relevance 15%]

* Re: [PATCH] Add worker_exec configuration option V2
  2017-03-09 19:41   ` [PATCH] Add worker_exec configuration option V2 Jeremy Evans
@ 2017-03-10 21:19     ` Eric Wong
  2017-03-11  5:26       ` Jeremy Evans
  0 siblings, 1 reply; 12+ results
From: Eric Wong @ 2017-03-10 21:19 UTC (permalink / raw)
  To: Jeremy Evans; +Cc: unicorn-public

Jeremy Evans <code@jeremyevans.net> wrote:
> Here's V2 of the patch, which I think should address all of the issues
> you pointed out.

Thanks.  Pushed to git://80x24.org/unicorn worker_exec I can add
tests, later, or you can.  I also had some FreeBSD test fixes
(which might apply to OpenBSD) on a VM somewhere which I'll Cc:
you on: there was also just SO_KEEPALIVE fix I posted:

  https://bogomips.org/unicorn-public/20170310203431.28067-1-e@80x24.org/raw

>      worker_nr = -1
>      until (worker_nr += 1) == @worker_processes
>        @workers.value?(worker_nr) and next
>        worker = Unicorn::Worker.new(worker_nr)
>        before_fork.call(self, worker)
> -      if pid = fork
> -        @workers[pid] = worker
> -        worker.atfork_parent
> +
> +      pid = if @worker_exec
> +        worker_spawn(worker)
>        else
> -        after_fork_internal
> -        worker_loop(worker)
> -        exit
> +        fork do
> +          after_fork_internal
> +          worker_loop(worker)
> +          exit
> +        end

I prefer to avoid the block with fork.  The block deepens the
stack for the running app, so it can affect GC efficiency.

Can be fixed in a separate patch...

^ permalink raw reply	[relevance 14%]

* Re: [PATCH] Add worker_exec configuration option V2
  2017-03-10 21:19     ` Eric Wong
@ 2017-03-11  5:26       ` Jeremy Evans
  2017-03-11  7:18         ` Eric Wong
  0 siblings, 1 reply; 12+ results
From: Jeremy Evans @ 2017-03-11  5:26 UTC (permalink / raw)
  To: Eric Wong; +Cc: unicorn-public

On 03/10 09:19, Eric Wong wrote:
> Jeremy Evans <code@jeremyevans.net> wrote:
> > Here's V2 of the patch, which I think should address all of the issues
> > you pointed out.
> 
> Thanks.  Pushed to git://80x24.org/unicorn worker_exec I can add
> tests, later, or you can.  I also had some FreeBSD test fixes
> (which might apply to OpenBSD) on a VM somewhere which I'll Cc:
> you on: there was also just SO_KEEPALIVE fix I posted:
> 
>   https://bogomips.org/unicorn-public/20170310203431.28067-1-e@80x24.org/raw

The C test code also returns 8 on OpenBSD, FWIW.  I'm happy to test any
test fixes on OpenBSD, just let me know.

> 
> >      worker_nr = -1
> >      until (worker_nr += 1) == @worker_processes
> >        @workers.value?(worker_nr) and next
> >        worker = Unicorn::Worker.new(worker_nr)
> >        before_fork.call(self, worker)
> > -      if pid = fork
> > -        @workers[pid] = worker
> > -        worker.atfork_parent
> > +
> > +      pid = if @worker_exec
> > +        worker_spawn(worker)
> >        else
> > -        after_fork_internal
> > -        worker_loop(worker)
> > -        exit
> > +        fork do
> > +          after_fork_internal
> > +          worker_loop(worker)
> > +          exit
> > +        end
> 
> I prefer to avoid the block with fork.  The block deepens the
> stack for the running app, so it can affect GC efficiency.
> 
> Can be fixed in a separate patch...

That makes sense.  If you would like me to send a separate patch to fix
it, I can do that.

Thanks,
Jeremy

^ permalink raw reply	[relevance 14%]

* Re: [PATCH] Add worker_exec configuration option V2
  2017-03-11  5:26       ` Jeremy Evans
@ 2017-03-11  7:18         ` Eric Wong
  2017-03-13 15:32           ` Jeremy Evans
  0 siblings, 1 reply; 12+ results
From: Eric Wong @ 2017-03-11  7:18 UTC (permalink / raw)
  To: Jeremy Evans; +Cc: unicorn-public

Jeremy Evans <code@jeremyevans.net> wrote:
> On 03/10 09:19, Eric Wong wrote:
> > tests, later, or you can.  I also had some FreeBSD test fixes
> > (which might apply to OpenBSD) on a VM somewhere which I'll Cc:
> > you on: there was also just SO_KEEPALIVE fix I posted:
> > 
> >   https://bogomips.org/unicorn-public/20170310203431.28067-1-e@80x24.org/raw
> 
> The C test code also returns 8 on OpenBSD, FWIW.  I'm happy to test any
> test fixes on OpenBSD, just let me know.

Thanks. I'll be happy to help fix any OpenBSD failures you see
from "gmake check"  I don't think the accept_filter fixes apply
to OpenBSD, but I guess the expr(1) fix did.

Thank you.

Same goes for NetBSD, DragonflyBSD or any other completely
Free/Open Source OSes anybody else here uses.

> > Jeremy Evans <code@jeremyevans.net> wrote:
> > > -      if pid = fork
> > > -        @workers[pid] = worker
> > > -        worker.atfork_parent
> > > +
> > > +      pid = if @worker_exec
> > > +        worker_spawn(worker)
> > >        else
> > > -        after_fork_internal
> > > -        worker_loop(worker)
> > > -        exit
> > > +        fork do
> > > +          after_fork_internal
> > > +          worker_loop(worker)
> > > +          exit
> > > +        end
> > 
> > I prefer to avoid the block with fork.  The block deepens the
> > stack for the running app, so it can affect GC efficiency.
> > 
> > Can be fixed in a separate patch...
> 
> That makes sense.  If you would like me to send a separate patch to fix
> it, I can do that.

Yes, please.

Not sure if we should automate the stack depth test...
I resisted it in the past since it might be too fragile
w.r.t changes to Ruby (even using "unicorn" via the
RubyGems-generated wrapper deepens it by 2).

Thanks again.

^ permalink raw reply	[relevance 13%]

* Re: [PATCH] Add worker_exec configuration option V2
  2017-03-11  7:18         ` Eric Wong
@ 2017-03-13 15:32           ` Jeremy Evans
  2017-03-13 19:18             ` Eric Wong
  0 siblings, 1 reply; 12+ results
From: Jeremy Evans @ 2017-03-13 15:32 UTC (permalink / raw)
  To: Eric Wong; +Cc: unicorn-public

On 03/11 07:18, Eric Wong wrote:
> Jeremy Evans <code@jeremyevans.net> wrote:
> > On 03/10 09:19, Eric Wong wrote:
> > > Jeremy Evans <code@jeremyevans.net> wrote:
> > > > -      if pid = fork
> > > > -        @workers[pid] = worker
> > > > -        worker.atfork_parent
> > > > +
> > > > +      pid = if @worker_exec
> > > > +        worker_spawn(worker)
> > > >        else
> > > > -        after_fork_internal
> > > > -        worker_loop(worker)
> > > > -        exit
> > > > +        fork do
> > > > +          after_fork_internal
> > > > +          worker_loop(worker)
> > > > +          exit
> > > > +        end
> > > 
> > > I prefer to avoid the block with fork.  The block deepens the
> > > stack for the running app, so it can affect GC efficiency.
> > > 
> > > Can be fixed in a separate patch...
> > 
> > That makes sense.  If you would like me to send a separate patch to fix
> > it, I can do that.
> 
> Yes, please.
> 
> Not sure if we should automate the stack depth test...
> I resisted it in the past since it might be too fragile
> w.r.t changes to Ruby (even using "unicorn" via the
> RubyGems-generated wrapper deepens it by 2).

Here's a patch to fix the stack depth issue:

From aa4914846a0870e5b01530a47d6dbfe09aeb39ce Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Mon, 13 Mar 2017 08:28:54 -0700
Subject: [PATCH] Don't pass a block for fork when forking workers

This reduces the stack depth, making GC more efficient.
---
 lib/unicorn/http_server.rb | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index a5bd2c4..023df10 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -541,14 +541,12 @@ def spawn_missing_workers
       worker = Unicorn::Worker.new(worker_nr)
       before_fork.call(self, worker)
 
-      pid = if @worker_exec
-        worker_spawn(worker)
-      else
-        fork do
-          after_fork_internal
-          worker_loop(worker)
-          exit
-        end
+      pid = @worker_exec ?  worker_spawn(worker) : fork
+      
+      unless pid
+        after_fork_internal
+        worker_loop(worker)
+        exit
       end
 
       @workers[pid] = worker
-- 
2.11.0


^ permalink raw reply	[relevance 13%]

* Re: [PATCH] Add worker_exec configuration option V2
  2017-03-13 15:32           ` Jeremy Evans
@ 2017-03-13 19:18             ` Eric Wong
  0 siblings, 0 replies; 12+ results
From: Eric Wong @ 2017-03-13 19:18 UTC (permalink / raw)
  To: Jeremy Evans; +Cc: unicorn-public

Jeremy Evans <code@jeremyevans.net> wrote:
> Here's a patch to fix the stack depth issue:

Thanks, applied with some trailing whitespace fixups.

(I suggest enabling the pre-commit hook distributed with
 git in $GIT_DIR/hooks/pre-commit, it'll run "git diff-index --check ..."
 to check that)

> From aa4914846a0870e5b01530a47d6dbfe09aeb39ce Mon Sep 17 00:00:00 2001

Btw, you can omit the above line and replace it with "---8<---"
(no quotes) so the applier can use "git am --scissors".

Thanks.

^ permalink raw reply	[relevance 14%]

* [ANN] unicorn 5.3.0.pre1 - Rack HTTP server for fast clients and Unix
@ 2017-03-24  0:28 Eric Wong
  0 siblings, 0 replies; 12+ results
From: Eric Wong @ 2017-03-24  0:28 UTC (permalink / raw)
  To: ruby-talk, unicorn-public; +Cc: Jeremy Evans, Simon Eskildsen

unicorn is an HTTP server for Rack applications designed to only serve
fast clients on low-latency, high-bandwidth connections and take
advantage of features in Unix/Unix-like kernels.  Slow clients should
only be served by placing a reverse proxy capable of fully buffering
both the the request and response in between unicorn and slow clients.

* https://bogomips.org/unicorn/
* public list: unicorn-public@bogomips.org
* mail archives: https://bogomips.org/unicorn-public/
* git clone git://bogomips.org/unicorn.git
* https://bogomips.org/unicorn/NEWS.atom.xml
* nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn

This is a pre-release RubyGem intended for testing.

Changes:

unicorn 5.3.0.pre1

A largish release with several new features.

Simon Eskildsen contributed heavily using TCP_INFO under Linux
to implement the (now 5 year old) check_client_connection feature:

  https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-check_client_connection
  https://bogomips.org/unicorn-public/?q=s:check_client_connection&d:..20170324&x=t

This also led to FreeBSD and OpenBSD portability improvements in
one of our dependencies, raindrops:

  https://bogomips.org/raindrops-public/20170323024829.GA5190@dcvr/T/#u

Jeremy Evans contributed several new features.  First he
implemented after_worker_exit to aid debugging:

  https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-after_worker_exit
  https://bogomips.org/unicorn-public/?q=s:after_worker_exit&d:..20170324&x=t#t

And then security-related features to isolate workers.  Workers
may now chroot to drop access to the master filesystem, and the
new after_worker_ready configuration hook now exists to aid with
chroot support in workers:

  https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-after_worker_ready
  https://bogomips.org/unicorn/Unicorn/Worker.html#method-i-user
  https://bogomips.org/unicorn-public/?q=s:after_worker_ready&d:..20170324&x=t#t
  https://bogomips.org/unicorn-public/?q=s:chroot&d:..20170324&x=t#t

Additionally, workers may run in a completely different VM space
(nullifying preload_app and any CoW savings) with the new
worker_exec option:

  https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-worker_exec
  https://bogomips.org/unicorn-public/?q=s:worker_exec&d:..20170324&x=t#t

There are also several improvements to FreeBSD and OpenBSD
support with the addition of these features.

34 changes since 5.2.0 (2016-10-31):

Eric Wong (27):
      drop rb_str_set_len compatibility replacement
      TUNING: document THP caveat for Linux users
      tee_input: simplify condition for IO#write
      remove response_start_sent
      http_request: freeze constant strings passed IO#write
      Revert "remove response_start_sent"
      t/t0012-reload-empty-config.sh: access ivars directly if needed
      t0011-active-unix-socket.sh: fix race condition in test
      new test for check_client_connection
      revert signature change to HttpServer#process_client
      support "struct tcp_info" on non-Linux and Ruby 2.2+
      unicorn_http: reduce rb_global_variable calls
      oob_gc: rely on opt_aref_with optimization on Ruby 2.2+
      http_request: reduce insn size for check_client_connection
      freebsd: avoid EINVAL when setting accept filter
      test-lib: expr(1) portability fix
      tests: keep disabled tests defined
      test_exec: SO_KEEPALIVE value only needs to be true
      doc: fix links to raindrops project
      http_request: support proposed Raindrops::TCP states on non-Linux
      ISSUES: expand on mail archive info + subscription disclaimer
      test_ccc: use a pipe to synchronize test
      doc: remove private email support address
      input: update documentation and hide internals.
      http_server: initialize @pid ivar
      gemspec: remove olddoc from build dependency
      doc: add version annotations for new features

Jeremy Evans (6):
      Add after_worker_exit configuration option
      Fix code example in after_worker_exit documentation
      Add support for chroot to Worker#user
      Add after_worker_ready configuration option
      Add worker_exec configuration option
      Don't pass a block for fork when forking workers

Simon Eskildsen (1):
      check_client_connection: use tcp state on linux

-- 
5.3.0 in a week, maybe?

^ permalink raw reply	[relevance 13%]

* [ANN] unicorn 5.3.0 - Rack HTTP server for fast clients and Unix
@ 2017-04-01  8:08 " Eric Wong
  0 siblings, 0 replies; 12+ results
From: Eric Wong @ 2017-04-01  8:08 UTC (permalink / raw)
  To: ruby-talk, unicorn-public
  Cc: Jeremy Evans, Simon Eskildsen, Dylan Thacker-Smith

unicorn is an HTTP server for Rack applications designed to only serve
fast clients on low-latency, high-bandwidth connections and take
advantage of features in Unix/Unix-like kernels.  Slow clients should
only be served by placing a reverse proxy capable of fully buffering
both the the request and response in between unicorn and slow clients.

* https://bogomips.org/unicorn/
* public list: unicorn-public@bogomips.org
* mail archives: https://bogomips.org/unicorn-public/
* git clone git://bogomips.org/unicorn.git
* https://bogomips.org/unicorn/NEWS.atom.xml
* nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn

Changes:

unicorn 5.3.0

A couple of portability fixes from Dylan Thacker-Smith and
Jeremy Evans since 5.3.0.pre1 over a week ago, but this looks
ready for a stable release, today.

When I started this over 8 years ago, I wondered if this would
just end up being an April Fools' joke.  Guess not.  I guess I
somehow tricked people into using a terribly marketed web server
that cannot talk directly to untrusted clients :x  Anyways,
unicorn won't be able to handle slow clients 8 years from now,
either, or 80 years from now.  And I vow never to learn to use
new-fangled things like epoll, kqueue, or threads :P

Anyways, this is a largish release with several new features,
and no backwards incompatibilities.

Simon Eskildsen contributed heavily using TCP_INFO under Linux
to implement the (now 5 year old) check_client_connection feature:

  https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-check_client_connection
  https://bogomips.org/unicorn-public/?q=s:check_client_connection&d:..20170401&x=t

This also led to FreeBSD and OpenBSD portability improvements in
one of our dependencies, raindrops:

   https://bogomips.org/raindrops-public/20170323024829.GA5190@dcvr/T/#u

Jeremy Evans contributed several new features.  First he
implemented after_worker_exit to aid debugging:

  https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-after_worker_exit
  https://bogomips.org/unicorn-public/?q=s:after_worker_exit&d:..20170401&x=t#t

And then security-related features to isolate workers.  Workers
may now chroot to drop access to the master filesystem, and the
new after_worker_ready configuration hook now exists to aid with
chroot support in workers:

  https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-after_worker_ready
  https://bogomips.org/unicorn/Unicorn/Worker.html#method-i-user
  https://bogomips.org/unicorn-public/?q=s:after_worker_ready&d:..20170401&x=t#t
  https://bogomips.org/unicorn-public/?q=s:chroot&d:..20170401&x=t#t

Additionally, workers may run in a completely different VM space
(nullifying preload_app and any CoW savings) with the new
worker_exec option:

  https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-worker_exec
  https://bogomips.org/unicorn-public/?q=s:worker_exec&d:..20170401&x=t#t

There are also several improvements to FreeBSD and OpenBSD
support with the addition of these features.

shortlog of changes since v5.2.0 (2016-10-31):

Dylan Thacker-Smith (1):
      Check for Socket::TCP_INFO constant before trying to get TCP_INFO

Eric Wong (30):
      drop rb_str_set_len compatibility replacement
      TUNING: document THP caveat for Linux users
      tee_input: simplify condition for IO#write
      remove response_start_sent
      http_request: freeze constant strings passed IO#write
      Revert "remove response_start_sent"
      t/t0012-reload-empty-config.sh: access ivars directly if needed
      t0011-active-unix-socket.sh: fix race condition in test
      new test for check_client_connection
      revert signature change to HttpServer#process_client
      support "struct tcp_info" on non-Linux and Ruby 2.2+
      unicorn_http: reduce rb_global_variable calls
      oob_gc: rely on opt_aref_with optimization on Ruby 2.2+
      http_request: reduce insn size for check_client_connection
      freebsd: avoid EINVAL when setting accept filter
      test-lib: expr(1) portability fix
      tests: keep disabled tests defined
      test_exec: SO_KEEPALIVE value only needs to be true
      doc: fix links to raindrops project
      http_request: support proposed Raindrops::TCP states on non-Linux
      ISSUES: expand on mail archive info + subscription disclaimer
      test_ccc: use a pipe to synchronize test
      doc: remove private email support address
      input: update documentation and hide internals.
      http_server: initialize @pid ivar
      gemspec: remove olddoc from build dependency
      doc: add version annotations for new features
      unicorn 5.3.0.pre1
      doc: note after_worker_exit is also 5.3.0+
      test_exec: SO_KEEPALIVE value only needs to be true (take #2)

Jeremy Evans (7):
      Add after_worker_exit configuration option
      Fix code example in after_worker_exit documentation
      Add support for chroot to Worker#user
      Add after_worker_ready configuration option
      Add worker_exec configuration option
      Don't pass a block for fork when forking workers
      Check for SocketError on first ccc attempt

Simon Eskildsen (1):
      check_client_connection: use tcp state on linux

-- 
Yes, this release is real despite the date.

^ permalink raw reply	[relevance 12%]

Results 1-12 of 12 | reverse results
2017-03-08 18:44 [PATCH] Add worker_exec configuration option Jeremy Evans
2017-03-08 20:02 ` Eric Wong
2017-03-09  4:52   ` Jeremy Evans
2017-03-09 13:57     ` Eric Wong
2017-03-09 19:41   ` [PATCH] Add worker_exec configuration option V2 Jeremy Evans
2017-03-10 21:19     ` Eric Wong
2017-03-11  5:26       ` Jeremy Evans
2017-03-11  7:18         ` Eric Wong
2017-03-13 15:32           ` Jeremy Evans
2017-03-13 19:18             ` Eric Wong
2017-03-24  0:28 [ANN] unicorn 5.3.0.pre1 - Rack HTTP server for fast clients and Unix Eric Wong
2017-04-01  8:08 [ANN] unicorn 5.3.0 " Eric Wong


unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://bogomips.org/unicorn-public
	git clone --mirror http://ou63pmih66umazou.onion/unicorn-public

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.unicorn

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git