The later exception wins

Author: rhenium

ext/openssl/ossl_bio.c

@@ -175,6 +175,56 @@ bio_bwrite0(VALUE args)
     }
 }
 
+struct call0_args {
+    VALUE (*func)(VALUE);
+    VALUE args;
+    VALUE ret;
+};
+
+static VALUE
+do_nothing(VALUE _)
+{
+    return Qnil;
+}
+
+static VALUE
+call_protect1(VALUE args_)
+{
+    struct call0_args *args = (void *)args_;
+    rb_set_errinfo(Qnil);
+    args->ret = args->func(args->args);
+    return Qnil;
+}
+
+static VALUE
+call_protect0(VALUE args_)
+{
+    /*
+     * At this point rb_errinfo() may be set by another callback called from
+     * the same OpenSSL function (e.g., SSL_accept()).
+     *
+     * Abusing rb_ensure() to temporarily save errinfo and restore it after
+     * the BIO callback successfully returns.
+     */
+    rb_ensure(do_nothing, Qnil, call_protect1, args_);
+    return Qnil;
+}
+
+static VALUE
+call_protect(VALUE (*func)(VALUE), VALUE args, int *state)
+{
+    /*
+     * FIXME: should check !NIL_P(rb_ivar_get(ssl_obj, ID_callback_state))
+     * instead to see if a tag jump is pending or not.
+     */
+    int pending = !NIL_P(rb_errinfo());
+    struct call0_args call0_args = { func, args, Qfalse };
+    rb_protect(call_protect0, (VALUE)&call0_args, state);
+    if (pending && *state)
+        rb_warn("exception ignored in BIO callback: pending=%d", pending);
+    return call0_args.ret;
+}
+
 static int
 bio_bwrite(BIO *bio, const char *data, int dlen)
 {
@@ -185,7 +235,7 @@ bio_bwrite(BIO *bio, const char *data, int dlen)
     if (ctx->state)
         return -1;
 
-    VALUE ok = rb_protect(bio_bwrite0, (VALUE)&args, &state);
+    VALUE ok = call_protect(bio_bwrite0, (VALUE)&args, &state);
     if (state) {
         ctx->state = state;
         return -1;
@@ -250,7 +300,7 @@ bio_bread(BIO *bio, char *data, int dlen)
     if (ctx->state)
         return -1;
 
-    VALUE ok = rb_protect(bio_bread0, (VALUE)&args, &state);
+    VALUE ok = call_protect(bio_bread0, (VALUE)&args, &state);
     if (state) {
         ctx->state = state;
         return -1;
@@ -280,7 +330,7 @@ bio_ctrl(BIO *bio, int cmd, long larg, void *parg)
       case BIO_CTRL_EOF:
         return ctx->eof;
       case BIO_CTRL_FLUSH:
-        rb_protect(bio_flush0, (VALUE)ctx, &state);
+        call_protect(bio_flush0, (VALUE)ctx, &state);
         ctx->state = state;
         return !state;
       default:

ext/openssl/ossl_ssl.c

@@ -1690,28 +1690,26 @@ ossl_ssl_setup(VALUE self)
 static void
 check_bio_error(VALUE self, SSL *ssl, VALUE bobj, int ret)
 {
-    VALUE cb_state = rb_attr_get(self, ID_callback_state);
-    if (!NIL_P(cb_state)) {
-        /* must cleanup OpenSSL error stack before re-raising */
-        ossl_clear_error();
-        rb_jump_tag(NUM2INT(cb_state));
-    }
-
-    // Socket BIO -> nothing to do
     if (NIL_P(bobj)) {
 #ifdef _WIN32
         errno = rb_w32_map_errno(WSAGetLastError());
 #endif
-        return;
     }
-
-    int state = ossl_bio_state(bobj);
-    if (!state) {
+    else {
+        int state = ossl_bio_state(bobj);
+        if (state) {
+            ossl_clear_error();
+            rb_jump_tag(state);
+        }
         errno = 0;
-        return;
     }
-    ossl_clear_error();
-    rb_jump_tag(state);
+
+    VALUE cb_state = rb_attr_get(self, ID_callback_state);
+    if (!NIL_P(cb_state)) {
+        /* must cleanup OpenSSL error stack before re-raising */
+        ossl_clear_error();
+        rb_jump_tag(NUM2INT(cb_state));
+    }
 }
 
 static void

test/openssl/test_ssl.rb

@@ -261,7 +261,7 @@ def test_synthetic_io_errors_in_servername_cb
   end
 
   def test_synthetic_io_errors_in_callback_and_socket
-    assert_separately(["-ropenssl"], <<~"end;")
+    assert_separately(["-ropenssl"], <<~"end;", ignore_stderr: true)
   begin
     #sock1, sock2 = socketpair
     sock1, sock2 = if defined? UNIXSocket
@@ -273,25 +273,33 @@ def test_synthetic_io_errors_in_callback_and_socket
     t = Thread.new {
       s1 = OpenSSL::SSL::SSLSocket.new(sock1)
       s1.hostname = "localhost"
-      assert_raise_with_message(OpenSSL::SSL::SSLError, /unrecognized.name/i) {
+      begin
         s1.connect
-      }
+      rescue
+      end
     }
 
+    called = []
     ctx2 = OpenSSL::SSL::SSLContext.new
     ctx2.servername_cb = lambda { |args|
-      throw :throw_from, :servername_cb
+      called << :servername_cb
+      raise "servername_cb"
     }
     obj = Object.new
     obj.define_singleton_method(:method_missing) { |name, *args, **kwargs| sock2.__send__(name, *args, **kwargs) }
     obj.define_singleton_method(:respond_to_missing?) { |name, *args, **kwargs| sock2.respond_to?(name, *args, **kwargs) }
     obj.define_singleton_method(:write_nonblock) { |*args, **kwargs|
-      raise "write_nonblock"
+      called << :write_nonblock
+      throw :throw_from, :write_nonblock
     }
     s2 = OpenSSL::SSL::SSLSocket.new(obj, ctx2)
 
-    ret = catch(:throw_from) { s2.accept }
-    assert_equal(:servername_cb, ret)
+    ret = assert_warning(/exception ignored/) {
+      catch(:throw_from) { s2.accept }
+    }
+    assert_equal(:write_nonblock, ret)
+    assert_equal([:servername_cb, :write_nonblock], called)
+    sock2.close
     assert t.join
   ensure
     sock1.close