Date | Commit message (Collapse) |
|
Once again Ruby seems ready to introduce more incompatibilities
and force busywork upon maintainers[1]. In order to avoid
incompatibilities in the future, I used a Perl script[2] to
prepend `frozen_string_literal: false' to every Ruby file.
Somebody interested will have to go through every Ruby source
file and enable frozen_string_literal once they've thoroughly
verified it's safe to do so.
[1] https://bugs.ruby-lang.org/issues/20205
[2] https://yhbt.net/add-fsl.git/74d7689/s/?b=add-fsl.perl
|
|
kgio is an extra download and shared object which costs users
bandwidth, disk space, startup time and memory. Ruby 2.3+
provides `Socket#accept_nonblock(exception: false)' support
in addition to `exception: false' support in IO#*_nonblock
methods from Ruby 2.1.
We no longer distinguish between TCPServer and UNIXServer as
separate classes internally; instead favoring the `Socket' class
of Ruby for both. This allows us to use `Socket#accept_nonblock'
and get a populated `Addrinfo' object off accept4(2)/accept(2)
without resorting to a getpeername(2) syscall (kgio avoided
getpeername(2) in the same way).
The downside is there's more Ruby-level argument passing and
stack usage on our end with HttpRequest#read_headers (formerly
HttpRequest#read). I chose this tradeoff since advancements in
Ruby itself can theoretically mitigate the cost of argument
passing, while syscalls are a high fixed cost given modern CPU
vulnerability mitigations.
Note: no benchmarks have been run since I don't have a suitable
system.
|
|
This means fewer redundant tests and more chances to notice
Ruby incompatibilities.
|
|
Less Ruby means fewer incompatibilities to worry about with
every new version.
|
|
No need to startup more processes than necessary.
|
|
...by testing less. `env["rack.input"].gets' users are out-of-luck
if they want anything other than "\n" or `nil', I suppose...
`$/' is non-thread-local and thus non-thread-safe, which doesn't
affect unicorn itself, but Ruby deprecates it for
single-threaded code, too, unfortunately.
Rack::Lint doesn't allow separator arguments for #gets, either,
so we can't support that, either...
|
|
The overread tests are ported over, and checksumming alone
is enough to guard against data corruption.
Randomizing the size of `read' calls on the client side will
shake out any boundary bugs on the server side.
|
|
t/integration.t already is more complete in that it tests
both Rack 2 and 3 along with both possible values of
check_client_connection.
|
|
We already have coverage for these basic things elsewhere.
|
|
The Perl 5 tests already rely on this implicitly, and there was
never a point when Perl 5 couldn't emulate systemd behavior.
|
|
http_response_write may benefit from API changes for Rack 3
support.
Since there's no benefit I can see from using a unit test,
switch to an integration test to avoid having to maintain the
unit test if our internal http_response_write method changes.
Of course, I can't trust tests written in Ruby since I've had to
put up with a constant stream of incompatibilities over the past
two decades :< Perl is more widely installed than socat[1], and
nearly all the Perl I wrote 20 years ago still works
unmodified today.
[1] the rarest dependency of the Bourne shell integration tests
|
|
Rack::Chunked will be gone in Rack 3.1, so provide a
non-middleware fallback which takes advantage of IO#write
supporting multiple arguments in Ruby 2.5+.
We still need to support Ruby 2.4, at least, since Rack 3.0
does. So a new (GC-unfriendly) Unicorn::WriteSplat module now
exists for Ruby <= 2.4 users.
|
|
Most changes are to the tests to avoid uppercase characters in header
keys, which are no longer allowed in rack 3 (to allow for O(1) access).
This also changes a few places where an array of headers was used to
switch to a hash, as a hash is requierd in Rack 3.
Newer versions of curl use a 000 http_code for invalid http codes,
so switch from "42 -eq" to "500 -ne" in the test, as Rack::Lint will
always raise a 500 error.
There is one test that fails on OpenBSD when opening a fifo. This is
unrelated to unicorn as far as I can see, so skip the remaining part
of the test in that case on OpenBSD.
Tests still pass on Rack 2, and presumably Rack 1 as well, though
I didn't test Rack 1.
Co-authored-by: Eric Wong <bofh@yhbt.net>
|
|
While the capabilities of epoll cannot be fully exploited given
our primitive design; avoiding thundering herd wakeups on larger
SMP machines while below 100% utilization is possible with
Linux 4.5+.
With this change, only one worker wakes up per-connect(2)
(instead of all of them via select(2)), avoiding the thundering
herd effect when the system is mostly idle.
Saturated instances should not notice the difference if they
rarely had multiple workers sleeping in select(2). This change
benefits non-saturated instances.
With 2 parallel clients and 8 workers on a nominally (:P)
8-core CPU (AMD FX-8320), the uconnect.perl test script
invocation showed a reduction from ~3.4s to ~2.5s when
reading an 11-byte response body:
echo worker_processes 8 >u.conf.rb
bs=11 ruby -I lib -I test/ruby-2.5.5/ext/unicorn_http/ bin/unicorn \
test/benchmark/dd.ru -E none -l /tmp/u.sock -c u.conf.rb
time perl -I lib -w test/benchmark/uconnect.perl \
-n 100000 -c 2 /tmp/u.sock
Times improve less as "-c" increases for uconnect.perl (system
noise and timings are inconsistent). The benefit of this change
should be more noticeable on systems with more workers (and
more cores).
I wanted to use EPOLLET (Edge-Triggered) to further reduce
syscalls, here, (similar to the old select()-avoidance bet) but
that would've either added too much complexity to deduplicate
wakeup sources, or run into the same starvation problem we
solved in April 2020[1].
Since the kernel already has the complexity and deduplication
built-in for Level-Triggered epoll support, we'll just let the
kernel deal with it.
Note: do NOT take this as an example of how epoll should be used
in a sophisticated server. unicorn is primitive by design and
cannot use threads nor handle multiple clients at once, thus it
it only uses epoll in this extremely limited manner.
Linux 4.5+ users will notice a regression of one extra epoll FD
per-worker and at least two epoll watches, so
/proc/sys/fs/epoll/max_user_watches may need to be changed along
with RLIMIT_NOFILE.
This change has also been tested on Linux 3.10.x (CentOS 7.x)
and FreeBSD 11.x to ensure compatibility with systems without
EPOLLEXCLUSIVE.
Various EPOLLEXCLUSIVE discussions over the years:
https://yhbt.net/lore/lkml/?q=s:EPOLLEXCLUSIVE+d:..20211001&x=t&o=-1
[1] https://yhbt.net/unicorn-public/CAMBWrQ=Yh42MPtzJCEO7XryVknDNetRMuA87irWfqVuLdJmiBQ@mail.gmail.com/
|
|
Ruby's handling of encodings hasn't changed much in over
a decade and these tests haven't failed for me since August 2013:
https://yhbt.net/unicorn-public/9af083d7f6b97c0f5ebbdd9a42b58478a6f874b7/s/
So lets take a small step in reducing energy consumption
and save potential developers over 10s of CPU time.
|
|
Otherwise we get test failures since we use sysread
and syswrite in many places
|
|
We don't want at_exit firing in child processes and never wanted
it. This is apparently a long standing bug in the tests that
only started causing test_worker_dies_on_dead_master failures
for me. I assume it's only showing up now for me due to kernel
scheduler changes, since I've been using the same 4-core CPU for
~11 years, now.
|
|
This adds `rack.after_reply` functionality which allows rack middleware
to pass lambdas that will be executed after the client connection has
been closed.
This was driven by a need to perform actions in a request that shouldn't
block the request from completing but also don't make sense as
background jobs.
There is prior art of this being supported found in a few gems, as well
as this functionality existing in other rack based servers (e.g. Puma).
[ew: check if `env' is set in ensure statement]
Acked-by: Eric Wong <e@80x24.org>
|
|
Ruby just recently bump the master version to 3.0.
This requirement bump is necessary to test unicorn
against ruby master.
[ew: wrap at <80 columns for hackers with poor eyesight]
Acked-by: Eric Wong <bofh@yhbt.net>
|
|
This can be useful for diagnosing failures, especially since
GNU tail supports inotify these days.
|
|
IO#sysread may only capture the 103 response and return before
the server can send the 200. Since we don't support persistent
connections, we can just use IO#read to rely on the server
giving us an EOF after the 200 is sent.
Cc: Jean Boussier <jean.boussier@gmail.com>
|
|
While not part of the rack spec, this API is exposed
by both puma and falcon, and Rails use it when available.
The 103 Early Hints response code is specified in RFC 8297.
|
|
We need to favor "Transfer-Encoding: chunked" over
"Content-Length" in the request header if they both exist.
Furthermore, we now reject redundant chunking and cases where
"chunked" is not the final encoding.
We currently do not and have no plans to decode "gzip",
"deflate", or "compress" encoding as described by RFC 7230.
That's a job more appropriate for middleware, anyways.
cf. https://tools.ietf.org/html/rfc7230
https://www.rfc-editor.org/errata_search.php?rfc=7230
|
|
We can start using Ruby 1.9 APIs, nowadays
|
|
It was added nearly 11 years ago in commit 6945342a1f0a4caa
("Transfer-Encoding: chunked streaming input support") but
never used.
|
|
My hardware gets worse and worse every year :<
|
|
Ruby 2.7.0dev warns on them
|
|
In preparation for kgio removal, I want to ensure we can
maintain existing performance when swapping kgio_tryaccept
for accept_nonblock on Ruby 2.3+
There's plenty of TCP benchmarking tools, but TCP port reuse
delays hurt predictability since unicorn doesn't do persistent
connections.
So this is exclusively for Unix sockets and uses Perl instead
of Ruby since I don't want to be bothered with GC
unpredictability on the client side.
|
|
This is intended to demonstrate how badly we suck at dealing
with slow clients making uploads. It can help users evaluate
alternative fully-buffering reverse proxies, because nginx
should not be the only option.
|
|
This is intended to demonstrate how badly we suck at dealing
with slow clients. It can help users evaluate alternative
fully-buffering reverse proxies, because nginx should not
be the only option.
Update the benchmark README while we're at it
|
|
String#-@ deduplicates strings starting with Ruby 2.5.0
Hash#[]= deduplicates strings starting in Ruby 2.6.0-rc1
This allows us to save a small amount of memory by sharing
objects with other parts of the stack (e.g. Rack).
|
|
We have never had any need for pipes with the default 64K
capacity on Linux. Our pipes are only used for tiny writes
in signal handlers and to perform parent shutdown detection.
With the current /proc/sys/fs/pipe-user-pages-soft
default, only 1024 pipes can be created by an unprivileged
user before Linux clamps down the pipe size to 4K (a single page)
for newly-created pipes[1].
So avoid penalizing OTHER pipe users who could benefit from the
increased capacity and use only a single page for ourselves.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/pipe.c?h=v4.18#n642
|
|
Ruby trunk started warning about more mismatched indentations
starting around r62836.
|
|
IO#wait_readable is introduced since 2.0
I confirmed we can pass tests for all versions of Ruby with this patch.
https://github.com/mtsmfm/unicorn/pull/2
|
|
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.
|
|
We need to ensure the portability of the sd_listen_fds emulation
test, too, which didn't get tested on my FreeBSD 10.3 install
due to it being on Ruby 2.2
Followup-to: 4ce6b00f75f1 ("test_exec: SO_KEEPALIVE value only needs to be true")
|
|
* ccc-tcp-v3:
test_ccc: use a pipe to synchronize test
http_request: support proposed Raindrops::TCP states on non-Linux
|
|
Sleeping 1 second to test 100 requests is too long for some
systems; and not long enough for others.
We need to also finish reading the sleeper response to ensure
the server actually got the second request in, before sending
SIGQUIT to terminate it; as it's possible for the test client
to connect and abort 100 clients before the server even
increments the request counter for the 2nd request.
|
|
* origin/ccc-tcp-v3:
http_request: reduce insn size for check_client_connection
support "struct tcp_info" on non-Linux and Ruby 2.2+
revert signature change to HttpServer#process_client
new test for check_client_connection
check_client_connection: use tcp state on linux
|
|
On FreeBSD 10.3, the value of SO_KEEPALIVE returned by
getsockopt is 8, even when set to '1' via setsockopt.
Relax the test to only ensure the boolean value is
interpreted as "true".
Verified independently of Ruby using the following:
--------8<---------
#include <sys/types.h>
#include <sys/socket.h>
#include <stdio.h>
static int err(const char *msg)
{
perror(msg);
return 1;
}
int main(void)
{
int sv[2];
int set = 1;
int got;
socklen_t len = (socklen_t)sizeof(int);
int rc;
rc = socketpair(PF_LOCAL, SOCK_STREAM, 0, sv);
if (rc) return err("socketpair failed");
rc = setsockopt(sv[0], SOL_SOCKET, SO_KEEPALIVE, &set, len);
if (rc) return err("setsockopt failed");
rc = getsockopt(sv[0], SOL_SOCKET, SO_KEEPALIVE, &got, &len);
if (rc) return err("getsockopt failed");
printf("got: %d\n", got);
return 0;
}
|
|
Some versions of test-unit will fail if an unspecified test is
attempted via "-n", so we need to define an empty test.
We cannot use "skip", either, as that seems exclusive to
minitest; and we won't use minitest since it has more
incompatible changes than test-unit over the last 8 years.
The memory leak test is gone since we're more versed in the
Ruby C API nowadays, modern GCs + mallocs may be less
predictable about releasing memory back to the OS.
|
|
We can force kgio_tryaccept to return an internal class
for TCP objects by subclassing Kgio::TCPServer.
This avoids breakage in any unfortunate projects which depend on
our undocumented internal APIs, such as gctools
<https://github.com/tmm1/gctools>
|
|
This was a bit tricky to test, but it's probably more reliable
now that we're relying on TCP_INFO.
Based on test by Simon Eskildsen <simon.eskildsen@shopify.com>:
https://bogomips.org/unicorn-public/CAO3HKM49+aLD=KLij3zhJqkWnR7bCWVan0mOvxD85xfrW8RXOw@mail.gmail.com/
|
|
* Use a frozen empty array and a class variable for TCP_Info to avoid
garbage. As far as I can tell, this shouldn't result in any garbage on
any requests (other than on the first request).
* Pass listener socket to #read to only check the client connection on
a TCP server.
* Short circuit CLOSE_WAIT after ESTABLISHED since in my testing it's
the most common state after ESTABLISHED, it makes the numbers
un-ordered, though. But comment should make it OK.
* Definition of of `check_client_connection` based on whether
Raindrops::TCP_Info is defined, instead of the class variable
approach.
* Changed the unit tests to pass a `nil` listener.
Tested on our staging environment, and still works like a dream.
I should note that I got the idea between this patch into Puma as well!
https://github.com/puma/puma/pull/1227
[ew: squashed in temporary change for oob_gc.rb, but we'll come
up with a different change to avoid breaking gctools
<https://github.com/tmm1/gctools>]
Acked-by: Eric Wong <e@80x24.org>
|
|
Oops, this was a half-baked change I was considering
but forgot about.
This reverts commit 69fd4f9bbff3708166fbf70163fa6e192dde1497.
|
|
|
|
Hi all.
We're implementing client certificate authentication with nginx and
unicorn.
Nginx configured in the following way:
proxy_set_header X-SSL-Client-Cert $ssl_client_cert;
When client submits certificate and nginx passes it to the unicorn,
unicorn responds with 400 (Bad Request). This caused because nginx
doesn't use "\r\n" they using just "\n" and multilne headers is failed
to parse (I've added test).
Accorording to RFC2616 section 19.3:
https://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html#sec19.3
"The line terminator for message-header fields is the sequence CRLF.
However, we recommend that applications, when parsing such headers,
recognize a single LF as a line terminator and ignore the leading CR."
CRLF changed to ("\r\n" | "\n")
Github commit
https://github.com/uno4ki/unicorn/commit/ed127b66e162aaf176de05720f6be758f8b41b1f
PS: Googling "nginx unicorn ssl_client_cert" shows the problem.
|
|
This provides some extra type safety if combined with other
C extensions, as well as allowing us to account for memory usage of
the HTTP parser in ObjectSpace.
This requires Ruby 1.9.3+ and has remained a stable API since
then. This will become officially supported when Ruby 2.3.0 is
released later this month.
This API has only been documented in doc/extension.rdoc (formerly
README.EXT) in the Ruby source tree since April 2015, r50318
|
|
This blatantly violates Rack SPEC, but we've had this bug since
March 2009[1]. Thus, we cannot expect all existing applications
and middlewares to fix this bug and will probably have to
support it forever.
Unfortunately, supporting this bug contributes to application
server lock-in, but at least we'll document it as such.
[1] commit 1835c9e2e12e6674b52dd80e4598cad9c4ea1e84
("HttpResponse: speed up non-multivalue headers")
Reported-by: Owen Ou <o@heroku.com>
Ref: <CAO47=rJa=zRcLn_Xm4v2cHPr6c0UswaFC_omYFEH+baSxHOWKQ@mail.gmail.com>
|
|
For some reason, I thought invalid descriptors passed to UNICORN_FD
would be automatically closed by the master process; but apparently
this hasn't been the case. On the other hand, this bug has been
around for over 6 years now and nobody noticed or cared enough to
tell us, so fixing it might break existing setups.
Since there may be users relying on this behavior, we cannot change
the behavior anymore; so update the documentation and write at test
to ensure we can never "fix" this bug at the expense of breaking
any working setups which may be out there.
Keep in mind that a before_exec hook may always be used to modify
the UNICORN_FD environment by setting the close_on_exec flag and
removing the appropriate descriptor from the environment.
I originally intended to add the ability to inherit new listeners
without a config file specification so systemd users can avoid
repeating themselves in the systemd and unicorn config files,
but apparently there is nothing to change in our code.
|