[ 
https://issues.apache.org/jira/browse/CRYPTO-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15866192#comment-15866192
 ] 

Marcelo Vanzin commented on CRYPTO-133:
---------------------------------------

There are hooks to make OpenSSL calls thread safe, and commons-crypto seems to 
install those hooks (and the code looked fine when I looked at it). But maybe 
there are issues in some specific environment, which is why I asked for that 
info; test works fine in my version even with the thread safety hooks removed.

Anyway, yes, it's hard to completely guarantee thread safety when you're 
depending on code you do not fully control for that. If there's some bug in 
some version of OpenSSL, there's not much that can be done (aside from making 
the Java method synchronized).

> OpenSslCryptoRandomNative.nextRandBytes not thread safe
> -------------------------------------------------------
>
>                 Key: CRYPTO-133
>                 URL: https://issues.apache.org/jira/browse/CRYPTO-133
>             Project: Commons Crypto
>          Issue Type: Bug
>            Reporter: Hendrik Saly
>
> Seems that AbstractRandomTest.testRandomBytesMultiThreaded is failing for 
> OpenSslCryptoRandomNative.nextRandBytes.
> Testcase throws exceptions like
> {code}
> java.lang.IllegalArgumentException: The nextRandBytes method failed
>       at 
> org.apache.commons.crypto.random.OpenSslCryptoRandom.nextBytes(OpenSslCryptoRandom.java:108)
>       at 
> org.apache.commons.crypto.random.AbstractRandomTest.checkRandomBytes(AbstractRandomTest.java:94)
>       at 
> org.apache.commons.crypto.random.AbstractRandomTest.access$000(AbstractRandomTest.java:30)
>       at 
> org.apache.commons.crypto.random.AbstractRandomTest$1.run(AbstractRandomTest.java:63)
> {code}
> When adding a 'synchronized' modifier to 
> OpenSslCryptoRandomNative.nextRandBytes it works.
> So IMHO there are two bugs that need to be resolved:
> 1) fix testcase AbstractRandomTest.testRandomBytesMultiThreaded in that way 
> that it fails when exception are thrown
> 2) fix OpenSslCryptoRandomNative.nextRandBytes no be thread safe (of course 
> not by adding 'synchronized', seems like locks_setup() is broken somehow in 
> https://github.com/apache/commons-crypto/blob/master/src/main/native/org/apache/commons/crypto/random/OpenSslCryptoRandomNative.c#L299
>  
> The testcase can be fixed with something like this
> {code}
>     @Test(timeout = 120000)
>     public void testRandomBytesMultiThreaded() throws Exception {
>         final int threadCount = 100;
>         final AtomicBoolean hasErrors = new AtomicBoolean();
>         try (final CryptoRandom random = getCryptoRandom()) {
>             final List<Thread> threads = new ArrayList<>(threadCount);
>             for (int i = 0; i < threadCount; i++) {
>                 Thread t = new Thread(new Runnable() {
>                     @Override
>                     public void run() {
>                         try {
>                                                       
> checkRandomBytes(random, 10);
>                                                       
> checkRandomBytes(random, 1000);
>                                                       
> checkRandomBytes(random, 100000);
>                                               } catch (Exception e) {
>                                                       hasErrors.set(true);
>                                                       e.printStackTrace();
>                                               }
>                     }
>                 });
>                 t.start();
>                 threads.add(t);
>             }
>             for (Thread t : threads) {
>                 if (!t.getState().equals(State.NEW)) {
>                     t.join();
>                 }
>             }
>             
>             if(hasErrors.get()) {
>               Assert.fail();
>             }
>         }
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to