AKI-668 fixes TxPoolV1 ConcurrentModificationException

aionnetwork · Mar 6, 2020 · 452d543 · 452d543
1 parent 40c14c0
commit 452d543
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 16 deletions.
diff --git a/modTxPool/src/org/aion/txpool/v1/TxPoolV1.java b/modTxPool/src/org/aion/txpool/v1/TxPoolV1.java
@@ -444,22 +444,46 @@ private BigInteger getAccountFirstPickingNonce(AionAddress sender) {
     public List<PooledTransaction> clearOutDateTransaction() {
         lock.lock();
         try {
-            List<PooledTransaction> clearedTransactions = new ArrayList<>();
-            Map<Long, Set<ByteArrayWrapper>> timeoutTxHashes = timeView.headMap(TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis()));
+            return clearOutDateTransaction(TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis()));
+        } finally {
+            lock.unlock();
+        }
+    }
 
-            for (Set<ByteArrayWrapper> hashes : timeoutTxHashes.values()) {
-                for (ByteArrayWrapper hash : hashes) {
-                    PooledTransaction pTx = poolRemove(hash);
-                    if (pTx != null) {
-                        clearedTransactions.add(pTx);
-                    }
+    List<PooledTransaction> clearOutDateTransaction(long outDateTime) {
+        List<PooledTransaction> clearedTransactions = new ArrayList<>();
+
+        for (Set<ByteArrayWrapper> set : timeView.headMap(outDateTime).values()) {
+            for (ByteArrayWrapper txHash : set) {
+                PooledTransaction removedTx = poolTransactions.remove(txHash);
+                if (removedTx == null) {
+                    LOG_TXPOOL.debug("Did not find the transaction hash:{} in the pool", txHash);
+                    continue;
                 }
-            }
 
-            return clearedTransactions;
-        } finally {
-            lock.unlock();
+                LOG_TXPOOL.debug("Removing tx[{}]", removedTx.tx);
+
+                Set<ByteArrayWrapper> feeSet = feeView.get(removedTx.tx.getEnergyPrice());
+                feeSet.remove(txHash);
+                if (feeSet.isEmpty()) {
+                    feeView.remove(removedTx.tx.getEnergyPrice());
+                }
+
+                SortedMap<BigInteger, ByteArrayWrapper> accountInfo =
+                    accountView.get(removedTx.tx.getSenderAddress());
+
+                accountInfo.remove(removedTx.tx.getNonceBI());
+                if (accountInfo.isEmpty()) {
+                    accountView.remove(removedTx.tx.getSenderAddress());
+                }
+
+                clearedTransactions.add(removedTx);
+                LOG_TXPOOL.debug("Removed tx[{}]", removedTx.tx);
+            }
         }
+        timeView.headMap(outDateTime).clear();
+
+        return clearedTransactions;
     }
 
     /**
@@ -623,10 +647,8 @@ public List<Long> getFeeList() {
     public List<AionTransaction> snapshot(long outDateTime) {
         lock.lock();
         try {
-            Map<Long, Set<ByteArrayWrapper>> timeoutTxHashes = timeView.headMap(outDateTime);
-
-            for (Set<ByteArrayWrapper> hashes : timeoutTxHashes.values()) {
-                for (ByteArrayWrapper hash : hashes) {
+            for (Set<ByteArrayWrapper> hashes : new ArrayList<>(timeView.headMap(outDateTime).values())) {
+                for (ByteArrayWrapper hash : new ArrayList<>(hashes)) {
                     PooledTransaction pTx = poolRemove(hash);
                     if (pTx == null) {
                         throw new IllegalStateException("The pool data has broken, cannot find the txHash:" + hash);

diff --git a/modTxPool/test/org/aion/txpool/v1/TxPoolV1Test.java b/modTxPool/test/org/aion/txpool/v1/TxPoolV1Test.java
@@ -1223,4 +1223,26 @@ public void getDroppedTxTest() {
         assertEquals(pooledTx, drppedPtx);
 
     }
+
+    @Test
+    public void clearOutDateTransactionTest() {
+        Properties config = new Properties();
+        config.put(TXPOOL_PROPERTY.PROP_TX_TIMEOUT, "10"); // 10 sec
+
+        TxPoolV1 tp = new TxPoolV1(config);
+        List<PooledTransaction> txl = new ArrayList<>();
+        int cnt = 2;
+        for (int i = 0; i < cnt; i++) {
+            PooledTransaction tx =
+                genTransaction(BigInteger.valueOf(i).toByteArray(), Constant.MIN_ENERGY_CONSUME);
+            txl.add(tx);
+        }
+        tp.add(txl);
+
+        tp.clearOutDateTransaction(TimeUnit.MICROSECONDS.toSeconds(txl.get(0).tx.getTimeStampBI().longValue()) + 10);
+        Assert.assertEquals(cnt, tp.size());
+
+        tp.clearOutDateTransaction(TimeUnit.MICROSECONDS.toSeconds(txl.get(0).tx.getTimeStampBI().longValue()) + 11);
+        Assert.assertEquals(0, tp.size());
+    }
 }