* [PATCH] tests: cleanup some unused variable warnings
@ 2017-12-22 3:17 99% Eric Wong
0 siblings, 0 replies; 19+ results
From: Eric Wong @ 2017-12-22 3:17 UTC (permalink / raw)
To: unicorn-public
Add a new "check-warnings" target to the GNUmakefile to make
checking for this easier. Warnings aren't fatal, and newer
versions of Ruby tend to increase warnings.
---
GNUmakefile | 5 +++++
test/unit/test_droplet.rb | 2 +-
test/unit/test_request.rb | 20 ++++++++++----------
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/GNUmakefile b/GNUmakefile
index 51045d4..2505e1f 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -249,5 +249,10 @@ endif
$(PLACEHOLDERS):
echo olddoc_placeholder > $@
+check-warnings:
+ @(for i in $$(git ls-files '*.rb' bin | grep -v '^setup\.rb$$'); \
+ do $(RUBY) --disable-gems -d -W2 -c \
+ $$i; done) | grep -v '^Syntax OK$$' || :
+
.PHONY: .FORCE-GIT-VERSION-FILE doc $(T) $(slow_tests) man
.PHONY: test-install
diff --git a/test/unit/test_droplet.rb b/test/unit/test_droplet.rb
index 73cf38c..81ad82b 100644
--- a/test/unit/test_droplet.rb
+++ b/test/unit/test_droplet.rb
@@ -4,7 +4,7 @@
class TestDroplet < Test::Unit::TestCase
def test_create_many_droplets
now = Time.now.to_i
- tmp = (0..1024).map do |i|
+ (0..1024).each do |i|
droplet = Unicorn::Worker.new(i)
assert droplet.respond_to?(:tick)
assert_equal 0, droplet.tick
diff --git a/test/unit/test_request.rb b/test/unit/test_request.rb
index f0ccaf7..6cb0268 100644
--- a/test/unit/test_request.rb
+++ b/test/unit/test_request.rb
@@ -34,7 +34,7 @@ def test_options
assert_equal '', env['REQUEST_PATH']
assert_equal '', env['PATH_INFO']
assert_equal '*', env['REQUEST_URI']
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_absolute_uri_with_query
@@ -44,7 +44,7 @@ def test_absolute_uri_with_query
assert_equal '/x', env['REQUEST_PATH']
assert_equal '/x', env['PATH_INFO']
assert_equal 'y=z', env['QUERY_STRING']
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_absolute_uri_with_fragment
@@ -55,7 +55,7 @@ def test_absolute_uri_with_fragment
assert_equal '/x', env['PATH_INFO']
assert_equal '', env['QUERY_STRING']
assert_equal 'frag', env['FRAGMENT']
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_absolute_uri_with_query_and_fragment
@@ -66,7 +66,7 @@ def test_absolute_uri_with_query_and_fragment
assert_equal '/x', env['PATH_INFO']
assert_equal 'a=b', env['QUERY_STRING']
assert_equal 'frag', env['FRAGMENT']
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_absolute_uri_unsupported_schemes
@@ -83,7 +83,7 @@ def test_x_forwarded_proto_https
"Host: foo\r\n\r\n")
env = @request.read(client)
assert_equal "https", env['rack.url_scheme']
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_x_forwarded_proto_http
@@ -92,7 +92,7 @@ def test_x_forwarded_proto_http
"Host: foo\r\n\r\n")
env = @request.read(client)
assert_equal "http", env['rack.url_scheme']
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_x_forwarded_proto_invalid
@@ -101,7 +101,7 @@ def test_x_forwarded_proto_invalid
"Host: foo\r\n\r\n")
env = @request.read(client)
assert_equal "http", env['rack.url_scheme']
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_rack_lint_get
@@ -109,7 +109,7 @@ def test_rack_lint_get
env = @request.read(client)
assert_equal "http", env['rack.url_scheme']
assert_equal '127.0.0.1', env['REMOTE_ADDR']
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_no_content_stringio
@@ -143,7 +143,7 @@ def test_rack_lint_put
"abcde")
env = @request.read(client)
assert ! env.include?(:http_body)
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
def test_rack_lint_big_put
@@ -177,6 +177,6 @@ def client.kgio_read!(*args)
}
assert_nil env['rack.input'].read(bs)
env['rack.input'].rewind
- res = @lint.call(env)
+ assert_kind_of Array, @lint.call(env)
end
end
--
EW
^ permalink raw reply related [relevance 99%]
* [PATCH] avoid reusing env on hijack
2017-12-05 1:53 99% ` Eric Wong
@ 2017-12-16 1:49 99% ` Eric Wong
0 siblings, 0 replies; 19+ results
From: Eric Wong @ 2017-12-16 1:49 UTC (permalink / raw)
To: Sam Saffron; +Cc: unicorn-public
Eric Wong <e@80x24.org> wrote:
> Btw, did you get a chance to look at my patch? (I haven't :x)
Thanks for testing (privately confirmed).
I've added a test case and pushed the following out to unicorn.git
https://bogomips.org/unicorn.git/patch?id=30e3c6abe542c6a9f5955e1d65896a0c3bab534f
I think we're due for a release soonish, especially with Ruby 2.5
around the corner...
--------8<--------
Subject: [PATCH] avoid reusing env on hijack
Hijackers may capture and reuse `env' indefinitely, so we must
not use it in those cases for future requests. For non-hijack
requests, we continue to reuse the `env' object to reduce
memory recycling.
Reported-and-tested-by: Sam Saffron <sam.saffron@gmail.com>
---
ext/unicorn_http/unicorn_http.rl | 15 +++++++++++++++
lib/unicorn/http_request.rb | 1 +
lib/unicorn/http_response.rb | 5 +++--
lib/unicorn/http_server.rb | 3 +--
t/hijack.ru | 12 ++++++++++++
t/t0200-rack-hijack.sh | 23 ++++++++++++++++++++++-
6 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
index 357440b..283bfa2 100644
--- a/ext/unicorn_http/unicorn_http.rl
+++ b/ext/unicorn_http/unicorn_http.rl
@@ -26,6 +26,7 @@ void init_unicorn_httpdate(VALUE mark_ary);
#define UH_FL_HASHEADER 0x100
#define UH_FL_TO_CLEAR 0x200
#define UH_FL_RESSTART 0x400 /* for check_client_connection */
+#define UH_FL_HIJACK 0x800
/* all of these flags need to be set for keepalive to be supported */
#define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER)
@@ -607,6 +608,10 @@ static VALUE HttpParser_clear(VALUE self)
{
struct http_parser *hp = data_get(self);
+ /* we can't safely reuse .buf and .env if hijacked */
+ if (HP_FL_TEST(hp, HIJACK))
+ return HttpParser_init(self);
+
http_parser_init(hp);
my_hash_clear(hp->env);
@@ -813,6 +818,15 @@ static VALUE HttpParser_env(VALUE self)
return data_get(self)->env;
}
+static VALUE HttpParser_hijacked_bang(VALUE self)
+{
+ struct http_parser *hp = data_get(self);
+
+ HP_FL_SET(hp, HIJACK);
+
+ return self;
+}
+
/**
* call-seq:
* parser.filter_body(dst, src) => nil/src
@@ -947,6 +961,7 @@ void Init_unicorn_http(void)
rb_define_method(cHttpParser, "next?", HttpParser_next, 0);
rb_define_method(cHttpParser, "buf", HttpParser_buf, 0);
rb_define_method(cHttpParser, "env", HttpParser_env, 0);
+ rb_define_method(cHttpParser, "hijacked!", HttpParser_hijacked_bang, 0);
rb_define_method(cHttpParser, "response_start_sent=", HttpParser_rssset, 1);
rb_define_method(cHttpParser, "response_start_sent", HttpParser_rssget, 0);
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index f83a566..d713b19 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -98,6 +98,7 @@ def read(socket)
# for rack.hijack, we respond to this method so no extra allocation
# of a proc object
def call
+ hijacked!
env['rack.hijack_io'] = env['unicorn.socket']
end
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index ec128e4..b23e521 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -21,13 +21,13 @@ def err_response(code, response_start_sent)
# writes the rack_response to socket as an HTTP response
def http_response_write(socket, status, headers, body,
- response_start_sent=false)
+ req = Unicorn::HttpRequest.new)
hijack = nil
if headers
code = status.to_i
msg = STATUS_CODES[code]
- start = response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
+ start = req.response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
buf = "#{start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \
"Date: #{httpdate}\r\n" \
"Connection: close\r\n"
@@ -52,6 +52,7 @@ def http_response_write(socket, status, headers, body,
end
if hijack
+ req.hijacked!
hijack.call(socket)
else
body.each { |chunk| socket.write(chunk) }
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index f33aa25..8674729 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -614,8 +614,7 @@ def process_client(client)
return if @request.hijacked?
end
@request.headers? or headers = nil
- http_response_write(client, status, headers, body,
- @request.response_start_sent)
+ http_response_write(client, status, headers, body, @request)
ensure
body.respond_to?(:close) and body.close
end
diff --git a/t/hijack.ru b/t/hijack.ru
index 4adec61..02260e2 100644
--- a/t/hijack.ru
+++ b/t/hijack.ru
@@ -11,11 +11,15 @@ def close
warn "closed DieIfUsed #{@@n += 1}\n"
end
end
+
+envs = []
+
run lambda { |env|
case env["PATH_INFO"]
when "/hijack_req"
if env["rack.hijack?"]
io = env["rack.hijack"].call
+ envs << env
if io.respond_to?(:read_nonblock) &&
env["rack.hijack_io"].respond_to?(:read_nonblock)
@@ -33,11 +37,19 @@ def close
{
"Content-Length" => r.bytesize.to_s,
"rack.hijack" => proc do |io|
+ envs << env
io.write(r)
io.close
end
},
DieIfUsed.new
]
+ when "/normal_env_id"
+ b = "#{env.object_id}\n"
+ h = {
+ 'Content-Type' => 'text/plain',
+ 'Content-Length' => b.bytesize.to_s,
+ }
+ [ 200, h, [ b ] ]
end
}
diff --git a/t/t0200-rack-hijack.sh b/t/t0200-rack-hijack.sh
index de3eb82..fee0791 100755
--- a/t/t0200-rack-hijack.sh
+++ b/t/t0200-rack-hijack.sh
@@ -1,6 +1,6 @@
#!/bin/sh
. ./test-lib.sh
-t_plan 5 "rack.hijack tests (Rack 1.5+ (Rack::VERSION >= [ 1,2]))"
+t_plan 9 "rack.hijack tests (Rack 1.5+ (Rack::VERSION >= [ 1,2]))"
t_begin "setup and start" && {
unicorn_setup
@@ -8,14 +8,35 @@ t_begin "setup and start" && {
unicorn_wait_start
}
+t_begin "normal env reused between requests" && {
+ env_a="$(curl -sSf http://$listen/normal_env_id)"
+ b="$(curl -sSf http://$listen/normal_env_id)"
+ test x"$env_a" = x"$b"
+}
+
t_begin "check request hijack" && {
test "xrequest.hijacked" = x"$(curl -sSfv http://$listen/hijack_req)"
}
+t_begin "env changed after request hijack" && {
+ env_b="$(curl -sSf http://$listen/normal_env_id)"
+ test x"$env_a" != x"$env_b"
+}
+
t_begin "check response hijack" && {
test "xresponse.hijacked" = x"$(curl -sSfv http://$listen/hijack_res)"
}
+t_begin "env changed after response hijack" && {
+ env_c="$(curl -sSf http://$listen/normal_env_id)"
+ test x"$env_b" != x"$env_c"
+}
+
+t_begin "env continues to be reused between requests" && {
+ b="$(curl -sSf http://$listen/normal_env_id)"
+ test x"$env_c" = x"$b"
+}
+
t_begin "killing succeeds after hijack" && {
kill $unicorn_pid
}
--
EW
^ permalink raw reply related [relevance 99%]
* Re: Auto scaling workers with unicorn
2017-12-05 1:51 99% ` Eric Wong
@ 2017-12-05 2:33 99% ` Ben Somers
0 siblings, 0 replies; 19+ results
From: Ben Somers @ 2017-12-05 2:33 UTC (permalink / raw)
To: Eric Wong; +Cc: Sam Saffron, unicorn-public
On Mon, Dec 4, 2017 at 5:51 PM, Eric Wong <e@80x24.org> wrote:
> Also, digging through the archives, Ben Somers came up with
> alicorn a while back and it might be up your alley:
>
> https://bogomips.org/unicorn-public/CAO1NZApo0TLJY2KgSg+Fjt1jEcuPfq=UCC0SCvvnuGDnr39w8w@mail.gmail.com/
AFAIK alicorn still works, but to my knowledge it stopped seeing
production use by anyone a couple years ago (the team I wrote it for
shut down, and I never heard of other production use). But it did do
the job with no incidents for several years, and the API's managed
carefully enough that it probably still works. At a minimum, it might be a
helpful starting point if you want to write your own tool.
^ permalink raw reply [relevance 99%]
* Re: env reuse and hijack
2017-11-29 1:03 99% ` Eric Wong
@ 2017-12-05 1:53 99% ` Eric Wong
2017-12-16 1:49 99% ` [PATCH] avoid reusing env on hijack Eric Wong
0 siblings, 1 reply; 19+ results
From: Eric Wong @ 2017-12-05 1:53 UTC (permalink / raw)
To: Sam Saffron; +Cc: unicorn-public
Eric Wong <e@80x24.org> wrote:
> Totally untested; but maybe the patch below works for you
> by replacing the buf and env objects in the hijacked cases.
>
> Can you try it and report back? Thanks.
Btw, did you get a chance to look at my patch? (I haven't :x)
^ permalink raw reply [relevance 99%]
* Re: Auto scaling workers with unicorn
2017-12-04 23:42 99% Auto scaling workers with unicorn Sam Saffron
@ 2017-12-05 1:51 99% ` Eric Wong
2017-12-05 2:33 99% ` Ben Somers
0 siblings, 1 reply; 19+ results
From: Eric Wong @ 2017-12-05 1:51 UTC (permalink / raw)
To: Sam Saffron; +Cc: unicorn-public
Sam Saffron <sam.saffron@gmail.com> wrote:
> I would like to amend Discourse so we "automatically" absorb certain
> traffic spikes. As it stands we can only configure unicorn with
> num_workers and use TTIN and TTOUT to tune the number on the fly.
>
> I was wondering if you would be open to patching unicorn to allow it
> to perform auto-tuning based on raindrops info.
I'm no fan of this or auto-tuning systems in general.
More explanation below.
> How it could work
>
> 1. configure unicorn with min_workers, max_workers, wince_delay, scale_up_delay
>
> 2. If queued requests is over 0 for N samples over scale_up_delay, add
> a worker up until max_workers
>
> 3. If queued requests is 0 for N samples over wince_delay scale down
> until you reach min workers
This adds more complexity to configuration: increasing the
likelyhood of getting these numbers completely wrong. GC and
malloc tuning is tricky and error-prone enough, already.
Mainly, this tends to hide problems for later; instead of
forcing you to deal with your resource limitations up front.
It becomes more difficult to forsee resource limitations down
the line. Before I worked on unicorn, I've seen auto-scaling
Apache workers mistuned far too often and running out of DB
connections or memory; and that happens at the worst time:
when your site is under heavy load (when you have the most to
lose (or gain)).
> Having this system in place can heavily optimize memory in large
> deployments and simplifies provisioning logic quite a lot.
My philosophy remains to tune for the worst case possible.
If you really need to do something like run an expensive
off-peak cronjob, maybe have it TTIN at the beginning and TTOU
again at the end.
Fwiw, the most useful thing I've found TTIN/TTOU for is cutting
down to one worker so I know which one to strace when tracking
down a problem; not auto-scaling.
> Wondering what you think about this and if you think unicorn should
> provide this option?
Fwiw, my position has been consistent on this throughout the years.
Also, digging through the archives, Ben Somers came up with
alicorn a while back and it might be up your alley:
https://bogomips.org/unicorn-public/CAO1NZApo0TLJY2KgSg+Fjt1jEcuPfq=UCC0SCvvnuGDnr39w8w@mail.gmail.com/
^ permalink raw reply [relevance 99%]
* Auto scaling workers with unicorn
@ 2017-12-04 23:42 99% Sam Saffron
2017-12-05 1:51 99% ` Eric Wong
0 siblings, 1 reply; 19+ results
From: Sam Saffron @ 2017-12-04 23:42 UTC (permalink / raw)
To: unicorn-public
I would like to amend Discourse so we "automatically" absorb certain
traffic spikes. As it stands we can only configure unicorn with
num_workers and use TTIN and TTOUT to tune the number on the fly.
I was wondering if you would be open to patching unicorn to allow it
to perform auto-tuning based on raindrops info.
How it could work
1. configure unicorn with min_workers, max_workers, wince_delay, scale_up_delay
2. If queued requests is over 0 for N samples over scale_up_delay, add
a worker up until max_workers
3. If queued requests is 0 for N samples over wince_delay scale down
until you reach min workers
Having this system in place can heavily optimize memory in large
deployments and simplifies provisioning logic quite a lot.
Wondering what you think about this and if you think unicorn should
provide this option?
^ permalink raw reply [relevance 99%]
* meta: any mailing list subscribers get duplicates?
@ 2017-11-29 2:02 99% Eric Wong
0 siblings, 0 replies; 19+ results
From: Eric Wong @ 2017-11-29 2:02 UTC (permalink / raw)
To: unicorn-public
If so, sorry about that.
This won't affect folks reading this list over NNTP or HTTP(S),
but looks like I forgot to lock in the replay driver and it should be
fixed in ssoma:
https://public-inbox.org/meta/20171129015224.31901-1-e@80x24.org/
(and yes, this is a test message of sorts, sorry for the noise)
As a reminder, you can also follow this list via NNTP and HTTP(S),
and it's probably more reliable since my MX may be blacklisted:
https://bogomips.org/unicorn-public/
http://ou63pmih66umazou.onion/unicorn-public/
nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn
nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.unicorn
/me resumes food coma
^ permalink raw reply [relevance 99%]
* Re: env reuse and hijack
2017-11-29 0:13 99% env reuse and hijack Sam Saffron
@ 2017-11-29 1:03 99% ` Eric Wong
2017-12-05 1:53 99% ` Eric Wong
0 siblings, 1 reply; 19+ results
From: Eric Wong @ 2017-11-29 1:03 UTC (permalink / raw)
To: Sam Saffron; +Cc: unicorn-public
Sam Saffron <sam.saffron@gmail.com> wrote:
> I was reading through unicorn today and noticed that it uses
> `HttpParser_clear` to clear up env between requests as opposed to
> allocating a new `env` object.
>
> This is generally fine, but if you hijack a request you may want to
> still look at env after this is done leading to situations where you
> are looking at the wrong env by the time you are dealing with the
> hijacked request
Right, unicorn has always reduced and avoided allocations by
recycling and reusing objects. Of course, rack.hijack was an
afterthought. (Zero-one-infinity design, and unicorn definitely
favors "one" for most things)
But obviously people use hijack nowadays with things like EM and
multiple envs/sockets exist...
> I guess I have 2 questions
>
> 1. Should Rack specify that env must be "re-initialized" if for any
> reason a request is hijacked?
*shrug* probably?
> 2. Should unicorn allow you to opt for env "recycle" via a rack key?
As in, adding a unicorn-specific API/option?
Then, no; (as usual) I don't want people writing unicorn-specific apps.
> I don't really have the answers here, my simplest course of action is
> simple to clone all the key/value pairs in env on to a new hash when I
> hijack, but it feels wasteful.
Right; allocating new things is wasteful either way; and you
seem to end up writing unicorn-specific code to clone this,
anyways...
So, the allocation/replacement should probably happen
transparently to users...
Totally untested; but maybe the patch below works for you
by replacing the buf and env objects in the hijacked cases.
Can you try it and report back? Thanks.
diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
index 357440b..283bfa2 100644
--- a/ext/unicorn_http/unicorn_http.rl
+++ b/ext/unicorn_http/unicorn_http.rl
@@ -26,6 +26,7 @@ void init_unicorn_httpdate(VALUE mark_ary);
#define UH_FL_HASHEADER 0x100
#define UH_FL_TO_CLEAR 0x200
#define UH_FL_RESSTART 0x400 /* for check_client_connection */
+#define UH_FL_HIJACK 0x800
/* all of these flags need to be set for keepalive to be supported */
#define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER)
@@ -607,6 +608,10 @@ static VALUE HttpParser_clear(VALUE self)
{
struct http_parser *hp = data_get(self);
+ /* we can't safely reuse .buf and .env if hijacked */
+ if (HP_FL_TEST(hp, HIJACK))
+ return HttpParser_init(self);
+
http_parser_init(hp);
my_hash_clear(hp->env);
@@ -813,6 +818,15 @@ static VALUE HttpParser_env(VALUE self)
return data_get(self)->env;
}
+static VALUE HttpParser_hijacked_bang(VALUE self)
+{
+ struct http_parser *hp = data_get(self);
+
+ HP_FL_SET(hp, HIJACK);
+
+ return self;
+}
+
/**
* call-seq:
* parser.filter_body(dst, src) => nil/src
@@ -947,6 +961,7 @@ void Init_unicorn_http(void)
rb_define_method(cHttpParser, "next?", HttpParser_next, 0);
rb_define_method(cHttpParser, "buf", HttpParser_buf, 0);
rb_define_method(cHttpParser, "env", HttpParser_env, 0);
+ rb_define_method(cHttpParser, "hijacked!", HttpParser_hijacked_bang, 0);
rb_define_method(cHttpParser, "response_start_sent=", HttpParser_rssset, 1);
rb_define_method(cHttpParser, "response_start_sent", HttpParser_rssget, 0);
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index f83a566..d713b19 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -98,6 +98,7 @@ def read(socket)
# for rack.hijack, we respond to this method so no extra allocation
# of a proc object
def call
+ hijacked!
env['rack.hijack_io'] = env['unicorn.socket']
end
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index ec128e4..b23e521 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -21,13 +21,13 @@ def err_response(code, response_start_sent)
# writes the rack_response to socket as an HTTP response
def http_response_write(socket, status, headers, body,
- response_start_sent=false)
+ req = Unicorn::HttpRequest.new)
hijack = nil
if headers
code = status.to_i
msg = STATUS_CODES[code]
- start = response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
+ start = req.response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
buf = "#{start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \
"Date: #{httpdate}\r\n" \
"Connection: close\r\n"
@@ -52,6 +52,7 @@ def http_response_write(socket, status, headers, body,
end
if hijack
+ req.hijacked!
hijack.call(socket)
else
body.each { |chunk| socket.write(chunk) }
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index f33aa25..8674729 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -614,8 +614,7 @@ def process_client(client)
return if @request.hijacked?
end
@request.headers? or headers = nil
- http_response_write(client, status, headers, body,
- @request.response_start_sent)
+ http_response_write(client, status, headers, body, @request)
ensure
body.respond_to?(:close) and body.close
end
^ permalink raw reply related [relevance 99%]
* env reuse and hijack
@ 2017-11-29 0:13 99% Sam Saffron
2017-11-29 1:03 99% ` Eric Wong
0 siblings, 1 reply; 19+ results
From: Sam Saffron @ 2017-11-29 0:13 UTC (permalink / raw)
To: unicorn-public
I was reading through unicorn today and noticed that it uses
`HttpParser_clear` to clear up env between requests as opposed to
allocating a new `env` object.
This is generally fine, but if you hijack a request you may want to
still look at env after this is done leading to situations where you
are looking at the wrong env by the time you are dealing with the
hijacked request
I guess I have 2 questions
1. Should Rack specify that env must be "re-initialized" if for any
reason a request is hijacked?
2. Should unicorn allow you to opt for env "recycle" via a rack key?
I don't really have the answers here, my simplest course of action is
simple to clone all the key/value pairs in env on to a new hash when I
hijack, but it feels wasteful.
^ permalink raw reply [relevance 99%]
* [PATCH] require 'pp' if $DEBUG is set by Rack app
2017-11-16 1:30 99% ` Eric Wong
@ 2017-11-16 21:09 99% ` Eric Wong
0 siblings, 0 replies; 19+ results
From: Eric Wong @ 2017-11-16 21:09 UTC (permalink / raw)
To: Robinson Jr, James P (Jim) HHHH; +Cc: unicorn-public
While "unicorn -d" requires 'pp' when setting $DEBUG, we did not
account for (rare) Rack applications setting $DEBUG at load time.
Thanks-to: James P (Jim) Robinson Jr <James.Robinson3@Cigna.com>
---
lib/unicorn.rb | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 4bd7bda..e7bc9ce 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -59,7 +59,10 @@ def self.builder(ru, op)
Object.const_get(File.basename(ru, '.rb').capitalize)
end
- pp({ :inner_app => inner_app }) if $DEBUG
+ if $DEBUG
+ require 'pp'
+ pp({ :inner_app => inner_app })
+ end
return inner_app if no_default_middleware
--
EW
^ permalink raw reply related [relevance 99%]
* Re: using pp without require 'pp' when $DEBUG = True
2017-11-16 0:57 99% using pp without require 'pp' when $DEBUG = True Robinson Jr, James P (Jim) HHHH
@ 2017-11-16 1:30 99% ` Eric Wong
2017-11-16 21:09 99% ` [PATCH] require 'pp' if $DEBUG is set by Rack app Eric Wong
0 siblings, 1 reply; 19+ results
From: Eric Wong @ 2017-11-16 1:30 UTC (permalink / raw)
To: Robinson Jr, James P (Jim) HHHH; +Cc: unicorn-public
"Robinson Jr, James P (Jim) HHHH" <James.Robinson3@Cigna.com> wrote:
> This is not an issue if you run "unicorn -d config.ru², but If you
> manually set $DEBUG = true, which I foolishly did because I didn¹t
> realize that $DEBUG was a special variable in ruby, you get an undefined
> method error.
Right, thanks for the bug report.
> I will be changing my code to use a different variable name, however I am
> not sure if there are valid reasons to manually set $DEBUG in ruby. If
> there are, this may be an issue for others.
Yes, $DEBUG is a special variable in Ruby, and documented in the
ruby manpage; anybody is allowed to set it.
> I think the simplest solution would be to replace the line above with an
> if block:
>
> if $DEBUG
> require 'pp'
> pp({ :inner_app => inner_app })
> end
Right, I'll prepare a patch tonight (gotta run out, soon) and
probably make a bugfix release in the next day or two, for this.
> Similar blocks appear in unicorn/bin/unicorn and
> unicorn/bin/unicorn_rails. I am assuming the one in unicorn/bin/unicorn
> is the one that allows it to work when using the -d flag. If there really
> is never any reason to manually set $DEBUG I¹m sorry I wasted your time.
>
> In any case, thank you for Unicorn!
No problem!
^ permalink raw reply [relevance 99%]
* using pp without require 'pp' when $DEBUG = True
@ 2017-11-16 0:57 99% Robinson Jr, James P (Jim) HHHH
2017-11-16 1:30 99% ` Eric Wong
0 siblings, 1 reply; 19+ results
From: Robinson Jr, James P (Jim) HHHH @ 2017-11-16 0:57 UTC (permalink / raw)
To: unicorn-public@bogomips.org
Hello,
I am not usually a ruby developer, and I just recently stumbled across
an odd bug, that only someone who didn¹t know what they were doing would
stumble across. I am honestly not sure if this really counts as a bug,
but I thought I should let you know.
unicorn/lib/unicorn.rb line 62 is:
pp({ :inner_app => inner_app }) if $DEBUG
Nowhere in the file is there require Œpp¹
This is not an issue if you run "unicorn -d config.ru², but If you
manually set $DEBUG = true, which I foolishly did because I didn¹t
realize that $DEBUG was a special variable in ruby, you get an undefined
method error.
I will be changing my code to use a different variable name, however I am
not sure if there are valid reasons to manually set $DEBUG in ruby. If
there are, this may be an issue for others.
I think the simplest solution would be to replace the line above with an
if block:
if $DEBUG
require 'pp'
pp({ :inner_app => inner_app })
end
Similar blocks appear in unicorn/bin/unicorn and
unicorn/bin/unicorn_rails. I am assuming the one in unicorn/bin/unicorn
is the one that allows it to work when using the -d flag. If there really
is never any reason to manually set $DEBUG I¹m sorry I wasted your time.
In any case, thank you for Unicorn!
Jim Robinson
------------------------------------------------------------------------------
CONFIDENTIALITY NOTICE: If you have received this email in error,
please immediately notify the sender by e-mail at the address shown.
This email transmission may contain confidential information. This
information is intended only for the use of the individual(s) or entity to
whom it is intended even if addressed incorrectly. Please delete it from
your files if you are not the intended recipient. Thank you for your
compliance. Copyright (c) 2017 Cigna
==============================================================================
^ permalink raw reply [relevance 99%]
* Re: Reaping process with unknown worker
2017-10-19 19:23 99% ` Alberto De Gaspari
@ 2017-10-19 19:37 99% ` Eric Wong
0 siblings, 0 replies; 19+ results
From: Eric Wong @ 2017-10-19 19:37 UTC (permalink / raw)
To: Alberto De Gaspari; +Cc: unicorn-public
Alberto De Gaspari <a.degaspari@18months.it> wrote:
> >> with a line like this in the log:
> >> INFO -- : reaped #<Process::Status: pid 16101 exit 0> worker=unknown
> >> if i run
> >> # ps axf |grep 16101
> >> i only get the ps line:
> >> 10277 pts/4 S+ 0:00 \_ grep 16101
> >>
> >> consider that i have loads of this line in the log, like at least 1
> >> every minute.
> >>
> >> what could cause those reaps?
> >
> > Reaping happens after a process exits, so it won't show up
> > in "ps" once it's reaped.
> >
> ok, got it:
> but if at t0 i run ps axf and save the result, when at t1 i find one of
> the lines in the stderr_log and try to grep the stated pid in the saved
> result of t0 i don't find anything.
I guess this is because the process is too short-lived. Can you
audit your code for when you spawn applications and check that?
Actually, what is curious is your master process is reaping
these processes (not workers reaping); yet your workers are not
dying and getting reaped.
Are you using preload_app?
If so, do you spawn any background processes at load time?
Or are there any background threads in the master process?
Does the problem go away if you disable preload_app?
Because, in normal applications, workers may spawn background
processes but its rare for the master to spawn anything but
workers themselves.
> how can i detect what it's reaping? do you consider this a normal
> behavior for a rails app which actually only receives simple get
> requests and save the result in a pg db?(that's what the small instance
> of unicorn does, the other is more complex, but shows the same amount
> and type of info messages in errors)
AFAIK, you cannot detect what it's reaping portably...
You can try the following hack to look for defunct (zombie)
processes before unicorn calls waitpid2. However, keep in mind
it is subject to race conditions so not 100% reliable:
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index f33aa25..f57271e 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -396,6 +396,7 @@ def awaken_master
# reaps all unreaped workers
def reap_all_workers
begin
+ system('ps | grep defunct')
wpid, status = Process.waitpid2(-1, Process::WNOHANG)
wpid or return
if @reexec_pid == wpid
One other thing you try do (on Linux) is strace the master:
strace -p $PID_OF_MASTER -f -e execve,clone
And see what the master is spawning. I suppose other OS has
similar functions (truss/ktruss/...)
^ permalink raw reply related [relevance 99%]
* Re: Reaping process with unknown worker
2017-10-19 19:05 99% ` Eric Wong
@ 2017-10-19 19:23 99% ` Alberto De Gaspari
2017-10-19 19:37 99% ` Eric Wong
0 siblings, 1 reply; 19+ results
From: Alberto De Gaspari @ 2017-10-19 19:23 UTC (permalink / raw)
To: Eric Wong; +Cc: unicorn-public
>> with a line like this in the log:
>> INFO -- : reaped #<Process::Status: pid 16101 exit 0> worker=unknown
>> if i run
>> # ps axf |grep 16101
>> i only get the ps line:
>> 10277 pts/4 S+ 0:00 \_ grep 16101
>>
>> consider that i have loads of this line in the log, like at least 1
>> every minute.
>>
>> what could cause those reaps?
>
> Reaping happens after a process exits, so it won't show up
> in "ps" once it's reaped.
>
ok, got it:
but if at t0 i run ps axf and save the result, when at t1 i find one of
the lines in the stderr_log and try to grep the stated pid in the saved
result of t0 i don't find anything.
how can i detect what it's reaping? do you consider this a normal
behavior for a rails app which actually only receives simple get
requests and save the result in a pg db?(that's what the small instance
of unicorn does, the other is more complex, but shows the same amount
and type of info messages in errors)
--
Cordiali saluti
Alberto De Gaspari
CTO & Software Engineer
18Months S.r.l
Via Copernico 38, Milano 20125, Italia
Tel: +39 02.92852164
Fax: +39 02.45075016
Mob: +39 349.1624385
MeetMe: doodle.com/albertodega
Email: a.degaspari@18months.it
web: www.18months.it
^ permalink raw reply [relevance 99%]
* Re: Reaping process with unknown worker
2017-10-19 18:46 99% ` Alberto De Gaspari
@ 2017-10-19 19:05 99% ` Eric Wong
2017-10-19 19:23 99% ` Alberto De Gaspari
0 siblings, 1 reply; 19+ results
From: Eric Wong @ 2017-10-19 19:05 UTC (permalink / raw)
To: Alberto De Gaspari; +Cc: unicorn-public
Alberto De Gaspari <a.degaspari@18months.it> wrote:
> >> Both the standard_error_logs are full of messages like the following:
> >>
> >> INFO -- : reaped #<Process::Status: pid 23835 exit 0> worker=unknown
> >>
> >> is this a misconfiguration or something similar?
> >
> > Does your application fork processes? It may not be reaping
> > them. Are your normal unicorn workers dying, too?
> no, workers never dies
> >
> > You can check the process tree or periodically run "ps" to see
> > what was running as the reaped pid before it died.
> >
> > I use "ps axf" from the "procps" package on some Linux sytems;
> > or "pstree" if installed to see the process tree.
> >
> i'm sorry but i don't get it.
I mean, run "ps axf" to get a list of all processes before they exit,
then check the output against the pid shown in the log when the
log entry eventually shows up.
> with a line like this in the log:
> INFO -- : reaped #<Process::Status: pid 16101 exit 0> worker=unknown
> if i run
> # ps axf |grep 16101
> i only get the ps line:
> 10277 pts/4 S+ 0:00 \_ grep 16101
>
> consider that i have loads of this line in the log, like at least 1
> every minute.
>
> what could cause those reaps?
Reaping happens after a process exits, so it won't show up
in "ps" once it's reaped.
^ permalink raw reply [relevance 99%]
* Re: Reaping process with unknown worker
2017-10-19 18:20 99% ` Eric Wong
@ 2017-10-19 18:46 99% ` Alberto De Gaspari
2017-10-19 19:05 99% ` Eric Wong
0 siblings, 1 reply; 19+ results
From: Alberto De Gaspari @ 2017-10-19 18:46 UTC (permalink / raw)
To: Eric Wong; +Cc: unicorn-public
>> Both the standard_error_logs are full of messages like the following:
>>
>> INFO -- : reaped #<Process::Status: pid 23835 exit 0> worker=unknown
>>
>> is this a misconfiguration or something similar?
>
> Does your application fork processes? It may not be reaping
> them. Are your normal unicorn workers dying, too?
no, workers never dies
>
> You can check the process tree or periodically run "ps" to see
> what was running as the reaped pid before it died.
>
> I use "ps axf" from the "procps" package on some Linux sytems;
> or "pstree" if installed to see the process tree.
>
i'm sorry but i don't get it.
with a line like this in the log:
INFO -- : reaped #<Process::Status: pid 16101 exit 0> worker=unknown
if i run
# ps axf |grep 16101
i only get the ps line:
10277 pts/4 S+ 0:00 \_ grep 16101
consider that i have loads of this line in the log, like at least 1
every minute.
what could cause those reaps?
^ permalink raw reply [relevance 99%]
* Re: Reaping process with unknown worker
2017-10-19 17:42 99% Reaping process with unknown worker Alberto De Gaspari
@ 2017-10-19 18:20 99% ` Eric Wong
2017-10-19 18:46 99% ` Alberto De Gaspari
0 siblings, 1 reply; 19+ results
From: Eric Wong @ 2017-10-19 18:20 UTC (permalink / raw)
To: Alberto De Gaspari; +Cc: unicorn-public
Alberto De Gaspari <a.degaspari@18months.it> wrote:
> Hello everyone, maybe this is a noob question, but it seems strange from
> my point of view, so here i am:
>
> I have 2 master processes running on the same server, pointing to the
> same rails application dir.
> I need 2 of them because i need some workers to be dedicated responding
> to requests coming from a specific socket(and i'm not sure this is the
> right way to do this)
That's fine.
> Both the standard_error_logs are full of messages like the following:
>
> INFO -- : reaped #<Process::Status: pid 23835 exit 0> worker=unknown
>
> is this a misconfiguration or something similar?
Does your application fork processes? It may not be reaping
them. Are your normal unicorn workers dying, too?
You can check the process tree or periodically run "ps" to see
what was running as the reaped pid before it died.
I use "ps axf" from the "procps" package on some Linux sytems;
or "pstree" if installed to see the process tree.
^ permalink raw reply [relevance 99%]
* Reaping process with unknown worker
@ 2017-10-19 17:42 99% Alberto De Gaspari
2017-10-19 18:20 99% ` Eric Wong
0 siblings, 1 reply; 19+ results
From: Alberto De Gaspari @ 2017-10-19 17:42 UTC (permalink / raw)
To: unicorn-public
Hello everyone, maybe this is a noob question, but it seems strange from
my point of view, so here i am:
I have 2 master processes running on the same server, pointing to the
same rails application dir.
I need 2 of them because i need some workers to be dedicated responding
to requests coming from a specific socket(and i'm not sure this is the
right way to do this)
Both the standard_error_logs are full of messages like the following:
INFO -- : reaped #<Process::Status: pid 23835 exit 0> worker=unknown
is this a misconfiguration or something similar?
Thank you in advance!
alberto
^ permalink raw reply [relevance 99%]
* Re: initialize/fork crash in macOS 10.13
@ 2017-10-16 19:25 99% ` Eric Wong
0 siblings, 0 replies; 19+ results
From: Eric Wong @ 2017-10-16 19:25 UTC (permalink / raw)
To: Jeffrey Carl Faden; +Cc: unicorn-public
Btw, this seems to be resolved in Ruby trunk and there's a
workaround documented for existing Rubies:
https://bugs.ruby-lang.org/issues/14009
Obviously I can't test it but it seems to be working for some people.
In case ruby-lang goes down, you can also find it from the
public-inbox mirror of ruby-core:
https://public-inbox.org/ruby-core/?q=%22Bug%20%2314009%22&x=t
^ permalink raw reply [relevance 99%]
Results 1-19 of 19 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2017-08-04 16:40 initialize/fork crash in macOS 10.13 Jeffrey Carl Faden
2017-08-04 19:10 ` Eric Wong
2017-10-16 19:25 99% ` Eric Wong
2017-10-19 17:42 99% Reaping process with unknown worker Alberto De Gaspari
2017-10-19 18:20 99% ` Eric Wong
2017-10-19 18:46 99% ` Alberto De Gaspari
2017-10-19 19:05 99% ` Eric Wong
2017-10-19 19:23 99% ` Alberto De Gaspari
2017-10-19 19:37 99% ` Eric Wong
2017-11-16 0:57 99% using pp without require 'pp' when $DEBUG = True Robinson Jr, James P (Jim) HHHH
2017-11-16 1:30 99% ` Eric Wong
2017-11-16 21:09 99% ` [PATCH] require 'pp' if $DEBUG is set by Rack app Eric Wong
2017-11-29 0:13 99% env reuse and hijack Sam Saffron
2017-11-29 1:03 99% ` Eric Wong
2017-12-05 1:53 99% ` Eric Wong
2017-12-16 1:49 99% ` [PATCH] avoid reusing env on hijack Eric Wong
2017-11-29 2:02 99% meta: any mailing list subscribers get duplicates? Eric Wong
2017-12-04 23:42 99% Auto scaling workers with unicorn Sam Saffron
2017-12-05 1:51 99% ` Eric Wong
2017-12-05 2:33 99% ` Ben Somers
2017-12-22 3:17 99% [PATCH] tests: cleanup some unused variable warnings Eric Wong
Code repositories for project(s) associated with this public inbox
https://yhbt.net/unicorn.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).