Forum: Ruby-core [ruby-trunk - Bug #8386][Open] OpenSSL thread safety

B012094b37ab6946c44eaa41d7828478?d=identicon&s=25 dbussink (Dirkjan Bussink) (Guest)
on 2013-05-10 13:12
(Received via mailing list)
Issue #8386 has been reported by dbussink (Dirkjan Bussink).

----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386

Author: dbussink (Dirkjan Bussink)
Status: Open
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
B012094b37ab6946c44eaa41d7828478?d=identicon&s=25 dbussink (Dirkjan Bussink) (Guest)
on 2013-05-10 13:13
(Received via mailing list)
Issue #8386 has been updated by dbussink (Dirkjan Bussink).

File openssl_thread_safety.patch added


----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-39241

Author: dbussink (Dirkjan Bussink)
Status: Open
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
303dd57f37d64288bb4f0336332a8882?d=identicon&s=25 ktsj (Kazuki Tsujimoto) (Guest)
on 2013-07-06 16:23
(Received via mailing list)
Issue #8386 has been updated by ktsj (Kazuki Tsujimoto).

File backtrace.txt added
Status changed from Closed to Assigned

=begin
Seems r41806 introduced a test failure under Ubuntu 13.04(x86_64):

 $ while :; do make TESTS='openssl -j2' test-all || cat; done
 [snip]
 /home/k_tsj/work/ruby/lib/test/unit/parallel.rb:37: [BUG]
vm_call0_cfunc_with_frame: cfp consistency error
 ruby 2.1.0dev (2013-07-06 trunk 41806) [x86_64-linux]

 -- Control frame information
-----------------------------------------------
 c:0004 p:---- s:0010 e:000009 CFUNC  :read
 c:0003 p:---- s:0008 e:000007 CFUNC  :new
 c:0002 p:0041 s:0005 e:000004 BLOCK
/home/k_tsj/work/ruby/lib/test/unit/parallel.rb:37 [FINISH]
 c:0001 p:---- s:0002 e:000001 TOP    [FINISH]

 /home/k_tsj/work/ruby/lib/test/unit/parallel.rb:37:in `block in
_run_suite'
 /home/k_tsj/work/ruby/lib/test/unit/parallel.rb:37:in `new'
 /home/k_tsj/work/ruby/lib/test/unit/parallel.rb:37:in `read'

I've attached the backtrace log.

Martin, can you check it?
=end

----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-40324

Author: dbussink (Dirkjan Bussink)
Status: Assigned
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
1ecef11b3cc6abfda85798858745ef72?d=identicon&s=25 MartinBosslet (Martin Bosslet) (Guest)
on 2013-07-06 16:39
(Received via mailing list)
Issue #8386 has been updated by MartinBosslet (Martin Bosslet).


ktsj (Kazuki Tsujimoto) wrote:
>
> I've attached the backtrace log.
>
> Martin, can you check it?

Sure, thanks for the heads up!


----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-40325

Author: dbussink (Dirkjan Bussink)
Status: Assigned
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
1ecef11b3cc6abfda85798858745ef72?d=identicon&s=25 MartinBosslet (Martin Bosslet) (Guest)
on 2013-07-08 02:06
(Received via mailing list)
Issue #8386 has been updated by MartinBosslet (Martin Bosslet).


Ok, this is gonna be fun... The error occurs in a test that triggers RSA
key generation, which was implemented to run with
'rb_thread_call_without_gvl'. The OpenSSL callbacks for thread safety
make use of rb_mutex_lock/rb_mutex_unlock. My initial guess is that
there's a problem because the calling code runs without the GVL, but I
need to analyze further to make a more educated guess.

It would help a lot if I had something isolated that reproduces the
error (at least with high probability).

@ktsj Does the error occur every time you run the tests in parallel? Or
only sporadically?
----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-40348

Author: dbussink (Dirkjan Bussink)
Status: Assigned
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
303dd57f37d64288bb4f0336332a8882?d=identicon&s=25 ktsj (Kazuki Tsujimoto) (Guest)
on 2013-07-09 16:16
(Received via mailing list)
Issue #8386 has been updated by ktsj (Kazuki Tsujimoto).


MartinBosslet (Martin Bosslet) wrote:
> @ktsj Does the error occur every time you run the tests in parallel? Or only
sporadically?

The latter (it is about 30-40% reproducible on my environment).


----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-40380

Author: dbussink (Dirkjan Bussink)
Status: Assigned
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
1ecef11b3cc6abfda85798858745ef72?d=identicon&s=25 MartinBosslet (Martin Bosslet) (Guest)
on 2013-07-10 10:10
(Received via mailing list)
Issue #8386 has been updated by MartinBosslet (Martin Bosslet).


ktsj (Kazuki Tsujimoto) wrote:
> MartinBosslet (Martin Bosslet) wrote:
> > @ktsj Does the error occur every time you run the tests in parallel? Or only
sporadically?
>
> The latter (it is about 30-40% reproducible on my environment).

Thanks! Ok, that seems good enough. I'll try to extract something in
isolation from that.

----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-40409

Author: dbussink (Dirkjan Bussink)
Status: Assigned
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
C4e88907313843cf07f6d85ba8162120?d=identicon&s=25 ko1 (Koichi Sasada) (Guest)
on 2013-07-23 12:12
(Received via mailing list)
Issue #8386 has been updated by ko1 (Koichi Sasada).


Maybe I fixed it.
Please check it.
----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-40621

Author: dbussink (Dirkjan Bussink)
Status: Closed
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
9361878d459f1709feec780518946ee5?d=identicon&s=25 naruse (Yui NARUSE) (Guest)
on 2013-07-24 12:17
(Received via mailing list)
Issue #8386 has been updated by naruse (Yui NARUSE).


ko1 (Koichi Sasada) wrote:
> Maybe I fixed it.
> Please check it.

I think dyn_lock_function and others also should be provided.
http://www.openssl.org/docs/crypto/threads.html
----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-40645

Author: dbussink (Dirkjan Bussink)
Status: Closed
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
1ecef11b3cc6abfda85798858745ef72?d=identicon&s=25 MartinBosslet (Martin Bosslet) (Guest)
on 2013-07-24 12:38
(Received via mailing list)
Issue #8386 has been updated by MartinBosslet (Martin Bosslet).


@ko1: Thank you so much for that! I think that functionality was exactly
what was needed. The error happens when RSA keys are created. The code
runs without the GVL in effect, and I believe the error was resulting
from the fact that a different native thread was running.

@naruse: You're right, and I also believe it would make sense to
distinguish between more "modes" in the callback. I'll try to give it
another try over the weekend. But please, if you have something
(planned) already, let me know and we could try to combine our efforts!
----------------------------------------
Bug #8386: OpenSSL thread safety
https://bugs.ruby-lang.org/issues/8386#change-40646

Author: dbussink (Dirkjan Bussink)
Status: Closed
Priority: Normal
Assignee: MartinBosslet (Martin Bosslet)
Category: ext
Target version:
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


As some might know, Rubinius uses a lot of MRI's C extensions that are
part of stdlib and ships them as well. Rubinius however does not have a
GIL, so thread safety issues in C extensions are much more important.

This patch fixes a thread safety issue in the OpenSSL extension, to
allow for it being used in a concurrent scenario. This follows from the
documentation of OpenSSL:

1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by
multiple threads). On Windows and many Unix systems, OpenSSL
automatically uses the multi-threaded versions of the standard
libraries. If your platform is not one of these, consult the INSTALL
file.

Multi-threaded applications must provide two callback functions to
OpenSSL by calling CRYPTO_set_locking_callback() and
CRYPTO_set_id_callback(), for all versions of OpenSSL up to and
including 0.9.8[abc...]. As of version 1.0.0, CRYPTO_set_id_callback()
and associated APIs are deprecated by CRYPTO_THREADID_set_callback() and
friends. This is described in the threads(3) manpage.

This patch adds using the callback and thread_id functions so this works
properly with Rubinius. This issue is not just theoretical, I've been
able to reproduce crashes when using OpenSSL in different threads that
have been fixed by this change. Rubinius already incorporated the
change, but preferable I would not have to maintain a custom fork with
these changes, but be able to use the MRI version without changes. I've
already discussed the change with Martin and he saw no reason for
objecting this change.
308cbef6e86dfc49cce3b2d4cf42aedc?d=identicon&s=25 SASADA Koichi (Guest)
on 2013-07-25 05:39
(Received via mailing list)
(2013/07/24 19:37), MartinBosslet (Martin Bosslet) wrote:
> @naruse: You're right, and I also believe it would make sense to distinguish
between more "modes" in the callback. I'll try to give it another try over the
weekend. But please, if you have something (planned) already, let me know and we
could try to combine our efforts!

I did. Please check it.
This topic is locked and can not be replied to.