Skip to content

Commit 0980748

Browse files
committed
Reduce OpenSSL::Buffering#do_write overhead
[Bug #20972] The `rb_str_new_freeze` was added in #452 to better handle concurrent use of a Socket, but SSL sockets can't be used concurrently AFAIK, so we might as well just error cleanly. By using `rb_str_locktmp` we can ensure attempts at concurrent write will raise an error, be we avoid causing a copy of the bytes. We also use the newer `String#append_as_bytes` method when available to save on some more copies. Co-Authored-By: luke.gru@gmail.com
1 parent f4e7c4b commit 0980748

File tree

2 files changed

+49
-28
lines changed

2 files changed

+49
-28
lines changed

ext/openssl/ossl_ssl.c

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2054,28 +2054,32 @@ ossl_ssl_read_nonblock(int argc, VALUE *argv, VALUE self)
20542054
}
20552055

20562056
static VALUE
2057-
ossl_ssl_write_internal(VALUE self, VALUE str, VALUE opts)
2057+
ossl_ssl_write_internal_safe(VALUE _args)
20582058
{
2059+
VALUE *args = (VALUE*)_args;
2060+
VALUE self = args[0];
2061+
VALUE str = args[1];
2062+
VALUE opts = args[2];
2063+
20592064
SSL *ssl;
20602065
rb_io_t *fptr;
20612066
int num, nonblock = opts != Qfalse;
2062-
VALUE tmp, cb_state;
2067+
VALUE cb_state;
20632068

20642069
GetSSL(self, ssl);
20652070
if (!ssl_started(ssl))
20662071
rb_raise(eSSLError, "SSL session is not started yet");
20672072

2068-
tmp = rb_str_new_frozen(StringValue(str));
20692073
VALUE io = rb_attr_get(self, id_i_io);
20702074
GetOpenFile(io, fptr);
20712075

20722076
/* SSL_write(3ssl) manpage states num == 0 is undefined */
2073-
num = RSTRING_LENINT(tmp);
2077+
num = RSTRING_LENINT(str);
20742078
if (num == 0)
20752079
return INT2FIX(0);
20762080

20772081
for (;;) {
2078-
int nwritten = SSL_write(ssl, RSTRING_PTR(tmp), num);
2082+
int nwritten = SSL_write(ssl, RSTRING_PTR(str), num);
20792083

20802084
cb_state = rb_attr_get(self, ID_callback_state);
20812085
if (!NIL_P(cb_state)) {
@@ -2116,6 +2120,21 @@ ossl_ssl_write_internal(VALUE self, VALUE str, VALUE opts)
21162120
}
21172121
}
21182122

2123+
static VALUE
2124+
ossl_ssl_write_internal(VALUE self, VALUE str, VALUE opts)
2125+
{
2126+
VALUE args[3] = {self, str, opts};
2127+
int state;
2128+
str = rb_str_locktmp(StringValue(str));
2129+
VALUE result = rb_protect(ossl_ssl_write_internal_safe, (VALUE)args, &state);
2130+
rb_str_unlocktmp(str);
2131+
2132+
if (state) {
2133+
rb_jump_tag(state);
2134+
}
2135+
return result;
2136+
}
2137+
21192138
/*
21202139
* call-seq:
21212140
* ssl.syswrite(string) => Integer

lib/openssl/buffering.rb

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,18 @@ module OpenSSL::Buffering
2323
include Enumerable
2424

2525
# A buffer which will retain binary encoding.
26-
class Buffer < String
27-
BINARY = Encoding::BINARY
28-
29-
def initialize
30-
super
31-
32-
force_encoding(BINARY)
33-
end
34-
35-
def << string
36-
if string.encoding == BINARY
37-
super(string)
38-
else
39-
super(string.b)
26+
if String.method_defined?(:append_as_bytes)
27+
Buffer = String
28+
else
29+
class Buffer < String
30+
def append_as_bytes(string)
31+
if string.encoding == Encoding::BINARY || string.ascii_only?
32+
self << string
33+
else
34+
self << string.b
35+
end
4036
end
41-
42-
return self
4337
end
44-
45-
alias concat <<
4638
end
4739

4840
##
@@ -352,22 +344,32 @@ def eof?
352344

353345
def do_write(s)
354346
@wbuffer = Buffer.new unless defined? @wbuffer
355-
@wbuffer << s
356-
@wbuffer.force_encoding(Encoding::BINARY)
347+
@wbuffer.append_as_bytes(s)
348+
357349
@sync ||= false
358-
buffer_size = @wbuffer.size
350+
buffer_size = @wbuffer.bytesize
359351
if @sync or buffer_size > BLOCK_SIZE
360352
nwrote = 0
361353
begin
362354
while nwrote < buffer_size do
363355
begin
364-
nwrote += syswrite(@wbuffer[nwrote, buffer_size - nwrote])
356+
chunk = if nwrote > 0
357+
@wbuffer.byteslice(nwrote..-1)
358+
else
359+
@wbuffer
360+
end
361+
362+
nwrote += syswrite(chunk)
365363
rescue Errno::EAGAIN
366364
retry
367365
end
368366
end
369367
ensure
370-
@wbuffer[0, nwrote] = ""
368+
if nwrote < @wbuffer.bytesize
369+
@wbuffer[0, nwrote] = ""
370+
else
371+
@wbuffer.clear
372+
end
371373
end
372374
end
373375
end

0 commit comments

Comments
 (0)