fix some small but annoying multiprocessing bugs

 * reduce (eliminate?) sporadic multiprocessing error messages at exit
 * Linux no longer requires green to be installed to run unit tests
 * also fixes gurnec#38

Author: gurnec

btcrecover.py

@@ -1,7 +1,7 @@
 #!/usr/bin/python
 
 # btcrecover.py -- Bitcoin wallet password recovery tool
-# Copyright (C) 2014-2016 Christopher Gurnee
+# Copyright (C) 2014-2017 Christopher Gurnee
 #
 # This program is free software: you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -28,7 +28,7 @@
 from __future__ import print_function
 
 from btcrecover import btcrpass
-import sys
+import sys, multiprocessing
 
 if __name__ == "__main__":
 
@@ -39,9 +39,17 @@
         btcrpass.safe_print("Password found: '" + password_found + "'")
         if any(ord(c) < 32 or ord(c) > 126 for c in password_found):
             print("HTML encoded:   '" + password_found.encode("ascii", "xmlcharrefreplace") + "'")
+        retval = 0
 
     elif not_found_msg:
         print(not_found_msg, file=sys.stderr if btcrpass.args.listpass else sys.stdout)
+        retval = 0
 
     else:
-        sys.exit(1)  # An error occurred or Ctrl-C was pressed
+        retval = 1  # An error occurred or Ctrl-C was pressed
+
+    # Wait for any remaining child processes to exit cleanly (to avoid error messages from gc)
+    for process in multiprocessing.active_children():
+        process.join(1.0)
+
+    sys.exit(retval)

btcrecover/btcrpass.py

@@ -29,7 +29,7 @@
 # (all optional futures for 2.7)
 from __future__ import print_function, absolute_import, division, unicode_literals
 
-__version__          =  "0.16.1"
+__version__          =  "0.16.2"
 __ordering_version__ = b"0.6.4"  # must be updated whenever password ordering changes
 
 import sys, argparse, itertools, string, re, multiprocessing, signal, os, cPickle, gc, \
@@ -5462,7 +5462,7 @@ def windows_ctrl_handler(signal):
 
     # Try to release as much memory as possible (before forking if multiple workers are being used)
     # (the initial counting process can be memory intensive)
-    gc.collect(2)
+    gc.collect()
 
     # Create an iterator which actually checks the (remaining) passwords produced by the password_iterator
     # by executing the return_verified_password_or_false worker function in possibly multiple threads
@@ -5500,6 +5500,7 @@ def windows_ctrl_handler(signal):
     try:
         for password_found, passwords_tried_last in password_found_iterator:
             if password_found:
+                if pool: pool.close()
                 passwords_tried += passwords_tried_last - 1  # just before the found password
                 if progress:
                     progress.next_update = 0  # force a screen update
@@ -5511,15 +5512,14 @@ def windows_ctrl_handler(signal):
             if l_savestate and passwords_tried % est_passwords_per_5min == 0:
                 do_autosave(args.skip + passwords_tried)
         else:  # if the for loop exits normally (without breaking)
-            if pool:
-                pool.close()
-                pool = None
+            if pool: pool.close()
             if progress:
                 if args.no_eta:
                     progress.maxval = passwords_tried
                 else:
                     progress.widgets.pop()  # remove the ETA
                 progress.finish()
+            if pool: pool.join()
 
     # Gracefully handle any exceptions, printing the count completed so far so that it can be
     # skipped if the user restarts the same run. If the exception was expected (Ctrl-C or some
@@ -5528,6 +5528,7 @@ def windows_ctrl_handler(signal):
     except BaseException as e:
         handled = handle_oom() if isinstance(e, MemoryError) and passwords_tried > 0 else False
         if not handled: print()  # move to the next line if handle_oom() hasn't already done so
+        if pool: pool.close()
 
         print("Interrupted after finishing password #", args.skip + passwords_tried, file=sys.stderr)
         if sys.stdout.isatty() ^ sys.stderr.isatty():  # if they're different, print to both to be safe
@@ -5542,5 +5543,4 @@ def windows_ctrl_handler(signal):
         do_autosave(args.skip + passwords_tried)
         autosave_file.close()
 
-    if pool: pool.terminate()
     return (password_found, "Password search exhausted" if password_found is False else None)

btcrecover/btcrseed.py

@@ -1161,8 +1161,8 @@ def config_mnemonic(self, mnemonic_guess = None, lang = None, passphrase = u"",
         passphrase = unicodedata.normalize("NFKD", passphrase)  # problematic w/Python narrow Unicode builds, same as Electrum
         passphrase = passphrase.lower()  # (?)
         passphrase = filter(lambda c: not unicodedata.combining(c), passphrase)  # remove combining marks
-        passphrase = u" ".join(passphrase.split())  # replace whitespace sequences with a single ANSI space
-        # remove ANSI whitespace between CJK characters (?)
+        passphrase = u" ".join(passphrase.split())  # replace whitespace sequences with a single ASCII space
+        # remove ASCII whitespace between CJK characters (?)
         passphrase = u"".join(c for i,c in enumerate(passphrase) if not (
                 c in string.whitespace
             and any(intvl[0] <= ord(passphrase[i-1]) <= intvl[1] for intvl in self.CJK_INTERVALS)

btcrecover/test/test_passwords.py

@@ -43,13 +43,13 @@
 warnings.filterwarnings("ignore", r"Not importing directory '.*gen_py': missing __init__.py", ImportWarning)
 
 from btcrecover import btcrpass
-import os, unittest, cPickle, tempfile, shutil, multiprocessing, filecmp, sys
+import os, unittest, cPickle, tempfile, shutil, multiprocessing, gc, filecmp, sys
 
 
 class NonClosingBase(object):
     pass
 
-# Enables either ANSI or Unicode mode for all tests based on either
+# Enables either ASCII or Unicode mode for all tests based on either
 # the value of tstr or the value of the BTCR_CHAR_MODE env. variable
 tstr = None
 def setUpModule():
@@ -72,7 +72,7 @@ def close(self): pass
         tstr = str
         tchr = chr
         utf8_opt = ""
-        print("** Testing in ANSI character mode **")
+        print("** Testing in ASCII character mode **")
 
     else:
         import io
@@ -918,9 +918,8 @@ def wallet_tester(self, wallet_filename,
                       force_purepython = False, force_kdf_purepython = False, force_bsddb_purepython = False,
                       correct_pass = None, blockchain_mainpass = None, android_backuppass = None):
         wallet_filename = os.path.join(WALLET_DIR, wallet_filename)
-
-        temp_dir = tempfile.mkdtemp("-test-btcr")
-        pool     = None
+        temp_dir        = tempfile.mkdtemp("-test-btcr")
+        parent_process  = True  # bug workaround, see finally block below for details
         try:
             temp_wallet_filename = os.path.join(temp_dir, os.path.basename(wallet_filename))
             shutil.copyfile(wallet_filename, temp_wallet_filename)
@@ -951,21 +950,26 @@ def wallet_tester(self, wallet_filename,
                 (tstr("btcr-wrong-password-3"), correct_pass, tstr("btcr-wrong-password-4"))), (correct_pass, 2))
 
             # Perform the tests in a child process to ensure the wallet can be pickled and all libraries reloaded
+            parent_process = False
             pool = multiprocessing.Pool(1, init_worker, (wallet, tstr, force_purepython, force_kdf_purepython))
+            parent_process = True
             password_found_iterator = pool.imap(btcrpass.return_verified_password_or_false,
                 ( ( tstr("btcr-wrong-password-1"), tstr("btcr-wrong-password-2") ),
                   ( tstr("btcr-wrong-password-3"), correct_pass, tstr("btcr-wrong-password-4") ) ))
             self.assertEqual(password_found_iterator.next(), (False, 2))
             self.assertEqual(password_found_iterator.next(), (correct_pass, 2))
             self.assertRaises(StopIteration, password_found_iterator.next)
             pool.close()
-            pool = None
+            pool.join()
 
             del wallet
+            gc.collect()
             self.assertTrue(filecmp.cmp(wallet_filename, temp_wallet_filename, False))  # False == always compare file contents
         finally:
-            shutil.rmtree(temp_dir)
-            if pool: pool.terminate()
+            # There's a bug which only occurs when combining unittest, multiprocessing, and "real"
+            # forking (Linux/BSD/WSL); only remove the temp dir if we're sure this is the parent process
+            if parent_process:
+                shutil.rmtree(temp_dir)
 
     def test_armory(self):
         if not can_load_armory(): self.skipTest("requires Armory and ASCII mode")
@@ -1003,9 +1007,11 @@ def test_electrum27_upgradedfrom_electrum1(self):
 
     @unittest.skipUnless(btcrpass.load_aes256_library().__name__ == b"Crypto", "requires PyCrypto")
     def test_electrum28(self):
+        if not can_load_armory(permit_unicode=True): self.skipTest("requires Armory")
         self.wallet_tester("electrum28-wallet")
 
     def test_electrum28_pp(self):
+        if not can_load_armory(permit_unicode=True): self.skipTest("requires Armory")
         self.wallet_tester("electrum28-wallet", force_purepython=True)
 
     @unittest.skipUnless(btcrpass.load_aes256_library().__name__ == b"Crypto", "requires PyCrypto")
@@ -1174,19 +1180,15 @@ def bip39_tester(self, force_purepython = False, unicode_pw = False, *args, **kw
             (tstr("btcr-wrong-password-3"), correct_pass, tstr("btcr-wrong-password-4"))), (correct_pass, 2))
 
         # Perform the tests in a child process to ensure the wallet can be pickled and all libraries reloaded
-        pool = None
-        try:
-            pool = multiprocessing.Pool(1, init_worker, (wallet, tstr, force_purepython, False))
-            password_found_iterator = pool.imap(btcrpass.return_verified_password_or_false,
-                ( ( tstr("btcr-wrong-password-1"), tstr("btcr-wrong-password-2") ),
-                  ( tstr("btcr-wrong-password-3"), correct_pass, tstr("btcr-wrong-password-4") ) ))
-            self.assertEqual(password_found_iterator.next(), (False, 2))
-            self.assertEqual(password_found_iterator.next(), (correct_pass, 2))
-            self.assertRaises(StopIteration, password_found_iterator.next)
-            pool.close()
-            pool = None
-        finally:
-            if pool: pool.terminate()
+        pool = multiprocessing.Pool(1, init_worker, (wallet, tstr, force_purepython, False))
+        password_found_iterator = pool.imap(btcrpass.return_verified_password_or_false,
+            ( ( tstr("btcr-wrong-password-1"), tstr("btcr-wrong-password-2") ),
+              ( tstr("btcr-wrong-password-3"), correct_pass, tstr("btcr-wrong-password-4") ) ))
+        self.assertEqual(password_found_iterator.next(), (False, 2))
+        self.assertEqual(password_found_iterator.next(), (correct_pass, 2))
+        self.assertRaises(StopIteration, password_found_iterator.next)
+        pool.close()
+        pool.join()
 
     @unittest.skipUnless(btcrpass.load_pbkdf2_library().__name__ == b"hashlib",
                          "requires Python 2.7.8+")
@@ -1696,6 +1698,8 @@ def test_end_to_end(self):
                                  data_extract         = self.E2E_DATA_EXTRACT,
                                  autosave             = autosave_file)
         self.assertEqual("btcr-test-password", btcrpass.main()[0])
+        for process in multiprocessing.active_children():
+            process.join()  # wait for any remaining child processes to exit cleanly
 
         # Verify the exact password number where it was found to ensure password ordering hasn't changed
         autosave_file.seek(SAVESLOT_SIZE)
@@ -1721,6 +1725,8 @@ def test_skip(self):
                                  data_extract         = self.E2E_DATA_EXTRACT,
                                  autosave             = autosave_file)
         self.assertIn("Password search exhausted", btcrpass.main()[1])
+        for process in multiprocessing.active_children():
+            process.join()  # wait for any remaining child processes to exit cleanly
 
         # Verify the password number where the search started
         autosave_file.seek(0)

run-all-tests.py

@@ -1,7 +1,7 @@
-#!/usr/bin/python
+#!/usr/bin/python
 
 # run-all-tests.py -- runs *all* btcrecover tests
-# Copyright (C) 2016 Christopher Gurnee
+# Copyright (C) 2016, 2017 Christopher Gurnee
 #
 # This file is part of btcrecover.
 #

seedrecover.py

@@ -1,7 +1,7 @@
-#!/usr/bin/python
+#!/usr/bin/python
 
 # seedrecover.py -- Bitcoin mnemonic sentence recovery tool
-# Copyright (C) 2014-2016 Christopher Gurnee
+# Copyright (C) 2014-2017 Christopher Gurnee
 #
 # This program is free software: you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -28,7 +28,7 @@
 from __future__ import print_function
 
 from btcrecover import btcrseed
-import sys
+import sys, multiprocessing
 
 if __name__ == "__main__":
 
@@ -46,7 +46,16 @@
         if btcrseed.tk_root:      # if the GUI is being used
             btcrseed.show_mnemonic_gui(mnemonic_sentence)
 
+        retval = 0
+
     elif mnemonic_sentence is None:
-        sys.exit(1)  # An error occurred or Ctrl-C was pressed inside btcrseed.main()
+        retval = 1  # An error occurred or Ctrl-C was pressed inside btcrseed.main()
+
+    else:
+        retval = 0  # "Seed not found" has already been printed to the console in btcrseed.main()
+
+    # Wait for any remaining child processes to exit cleanly (to avoid error messages from gc)
+    for process in multiprocessing.active_children():
+        process.join(1.0)
 
-    # else "Seed not found" has already been printed to the console in btcrseed.main()
+    sys.exit(retval)