[jira] [Commented] (CRYPTO-133) OpenSslCryptoRandomNative.nextRandBytes not thread safe

["Gary Gregory (JIRA)" <ji...@apache.org>]

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

Gary Gregory commented on CRYPTO-133:
-------------------------------------

Maybe what we need is clear Javadoc on what is thread-safe now. Then we can evolve from there and see if we want to make changes to make some code thread-safe. Otherwise, user's can just create separate objects for not thread-safe code.

> 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.4#6332)