From b496f81b207a55a1faef70b9c2e6cf2e5b7f52cf Mon Sep 17 00:00:00 2001
From: Christian Basler <chrigu.meyer@gmail.com>
Date: Tue, 27 Oct 2015 07:50:20 +0100
Subject: [PATCH] Fixed POWEngine and improved test. (Locks can't be released
 from a different thread, we need to use a semaphore)

---
 .../ports/MultiThreadedPOWEngine.java         | 19 ++++++++++++--
 .../ports/ProofOfWorkEngineTest.java          | 26 ++++++++++++++++---
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/domain/src/main/java/ch/dissem/bitmessage/ports/MultiThreadedPOWEngine.java b/domain/src/main/java/ch/dissem/bitmessage/ports/MultiThreadedPOWEngine.java
index f9db392..2907473 100644
--- a/domain/src/main/java/ch/dissem/bitmessage/ports/MultiThreadedPOWEngine.java
+++ b/domain/src/main/java/ch/dissem/bitmessage/ports/MultiThreadedPOWEngine.java
@@ -24,6 +24,7 @@ import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.Semaphore;
 
 import static ch.dissem.bitmessage.utils.Bytes.inc;
 
@@ -31,15 +32,28 @@ import static ch.dissem.bitmessage.utils.Bytes.inc;
  * A POW engine using all available CPU cores.
  */
 public class MultiThreadedPOWEngine implements ProofOfWorkEngine {
-    private static Logger LOG = LoggerFactory.getLogger(MultiThreadedPOWEngine.class);
+    private static final Logger LOG = LoggerFactory.getLogger(MultiThreadedPOWEngine.class);
+    private static final Semaphore semaphore = new Semaphore(1, true);
 
+    /**
+     * Although it has a callback, this method will block until all pending nonce calculations are done. (It gets very
+     * inefficient if multiple nonce are calculated at the same time.
+     *
+     * @param initialHash the SHA-512 hash of the object to send, sans nonce
+     * @param target      the target, representing an unsigned long
+     * @param callback    called with the calculated nonce as argument. The ProofOfWorkEngine implementation must make
+     */
     @Override
     public void calculateNonce(byte[] initialHash, byte[] target, Callback callback) {
+        try {
+            semaphore.acquire();
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
         callback = new CallbackWrapper(callback);
         int cores = Runtime.getRuntime().availableProcessors();
         if (cores > 255) cores = 255;
         LOG.info("Doing POW using " + cores + " cores");
-        long time = System.currentTimeMillis();
         List<Worker> workers = new ArrayList<>(cores);
         for (int i = 0; i < cores; i++) {
             Worker w = new Worker(workers, (byte) cores, i, initialHash, target, callback);
@@ -89,6 +103,7 @@ public class MultiThreadedPOWEngine implements ProofOfWorkEngine {
                             try {
                                 callback.onNonceCalculated(nonce);
                             } finally {
+                                semaphore.release();
                                 for (Worker w : workers) {
                                     w.interrupt();
                                 }
diff --git a/domain/src/test/java/ch/dissem/bitmessage/ports/ProofOfWorkEngineTest.java b/domain/src/test/java/ch/dissem/bitmessage/ports/ProofOfWorkEngineTest.java
index 38e52a8..b4fd194 100644
--- a/domain/src/test/java/ch/dissem/bitmessage/ports/ProofOfWorkEngineTest.java
+++ b/domain/src/test/java/ch/dissem/bitmessage/ports/ProofOfWorkEngineTest.java
@@ -25,12 +25,12 @@ import static ch.dissem.bitmessage.utils.Singleton.security;
 import static org.junit.Assert.assertTrue;
 
 public class ProofOfWorkEngineTest extends TestBase {
-    @Test
+    @Test(timeout = 90_000)
     public void testSimplePOWEngine() throws InterruptedException {
         testPOW(new SimplePOWEngine());
     }
 
-    @Test
+    @Test(timeout = 90_000)
     public void testThreadedPOWEngine() throws InterruptedException {
         testPOW(new MultiThreadedPOWEngine());
     }
@@ -49,8 +49,28 @@ public class ProofOfWorkEngineTest extends TestBase {
                     }
                 });
         byte[] nonce = waiter.waitForValue();
-        System.out.println("Calculating nonce took " + (System.currentTimeMillis() - time) + "ms");
+        time = System.currentTimeMillis() - time;
+        System.out.println("Calculating nonce took " + time + "ms");
         assertTrue(Bytes.lt(security().doubleSha512(nonce, initialHash), target, 8));
+
+        // Let's add a second (shorter) run to find possible multi threading issues
+        long time2 = System.currentTimeMillis();
+        byte[] initialHash2 = security().sha512(new byte[]{1, 3, 6, 5});
+        byte[] target2 = {0, -1, -1, -1, -1, -1, -1, -1};
+
+        final CallbackWaiter<byte[]> waiter2 = new CallbackWaiter<>();
+        engine.calculateNonce(initialHash2, target2,
+                new ProofOfWorkEngine.Callback() {
+                    @Override
+                    public void onNonceCalculated(byte[] nonce) {
+                        waiter2.setValue(nonce);
+                    }
+                });
+        byte[] nonce2 = waiter2.waitForValue();
+        time2 = System.currentTimeMillis() - time2;
+        System.out.println("Calculating nonce took " + time2 + "ms");
+        assertTrue(Bytes.lt(security().doubleSha512(nonce2, initialHash2), target2, 8));
+        assertTrue("Second nonce must be quicker to find", time > time2);
     }
 
 }