changes to thread code (directly use boost::thread) #2553

Diapolo commented at 9:40 AM on April 23, 2013: none

removes our NewThread() function an replaces these calls with boost::thread with our TraceThread template
make ThreadCleanWalletPassphrase() use an int64 as parameter
remove ExitThread() function
fix THREAD_PRIORITY_ABOVE_NORMAL for non Windows OSes

Diapolo commented at 9:48 AM on April 23, 2013: none

@BitcoinPullTester Error seems unrelated to my changes!

<pre> Exception in thread "main" java.lang.reflect.InvocationTargetException at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:616) at org.eclipse.jdt.internal.jarinjarloader.JarRsrcLoader.main(JarRsrcLoader.java:58) Caused by: java.lang.OutOfMemoryError: Java heap space at com.google.bitcoin.core.FullBlockTestGenerator.getBlocksToTest(FullBlockTestGenerator.java:1335) at com.google.bitcoin.core.BitcoindComparisonTool.<init>(BitcoindComparisonTool.java:77) at com.google.bitcoin.core.BitcoindComparisonTool.main(BitcoindComparisonTool.java:50) ... 5 more </pre>

TheBlueMatt commented at 1:45 PM on April 23, 2013: member

Oops, false-positive, sorry, retesting... also, tagging @BitcoinPullTester doesnt email me, please tag @TheBlueMatt instead

in src/rpcwallet.cpp:None in 490f76b1f7 outdated

1301 | @@ -1308,7 +1302,7 @@ void ThreadCleanWalletPassphrase(void* parg)
1302 |  
1303 |      LEAVE_CRITICAL_SECTION(cs_nWalletUnlockTime);
1304 |  
1305 | -    delete (int64*)parg;
1306 | +    delete (int64*)pnSleepTime;

gavinandresen commented at 3:43 PM on April 24, 2013:

cast isn't necessary, pnSleepTime is already an int64*

gavinandresen commented at 3:45 PM on April 24, 2013:

Also, the code would be simpler if an int64 was passed instead of an int64* -- the only reason a pointer was passed before was because NewThread was dumb and could only pass one pointer.

Diapolo commented at 4:45 PM on April 24, 2013:

I updated ThreadCleanWalletPassphrase() to use an int64, can you have another look?

BitcoinPullTester commented at 8:57 PM on April 26, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/7df7cf51daf0ac9b491a356222f7217f296b0ace for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

Diapolo commented at 4:04 PM on April 27, 2013: none

Any further comments on this?

BitcoinPullTester commented at 4:53 PM on April 30, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/7796fe9da178a92108682f05e9ac037012326cd9 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

in src/rpcwallet.cpp:None in 7796fe9da1 outdated

1333 | @@ -1342,9 +1334,10 @@ Value walletpassphrase(const Array& params, bool fHelp)
1334 |              "walletpassphrase <passphrase> <timeout>\n"
1335 |              "Stores the wallet decryption key in memory for <timeout> seconds.");
1336 |  
1337 | -    NewThread(ThreadTopUpKeyPool, NULL);
1338 | -    int64* pnSleepTime = new int64(params[1].get_int64());
1339 | -    NewThread(ThreadCleanWalletPassphrase, pnSleepTime);
1340 | +    boost::thread(boost::bind(&TraceThread<void (*)()>, "key-top", &ThreadTopUpKeyPool));
1341 | +    int64 pnSleepTime = params[1].get_int64();

sipa commented at 4:58 PM on April 30, 2013:

nit: remove the p in the name (I don't care much for the type-prefixes in variable names, but when you use them, they should be correct).

Diapolo commented at 9:05 AM on May 1, 2013: none

@sipa Fixed your nit ;) and you were absolutely right.

BitcoinPullTester commented at 9:27 AM on May 1, 2013: none

Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/1f52db4fdd3641e2c0ebbc2355ccc27547eaded7 for test log.

This pull does not merge cleanly onto current master This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

Diapolo commented at 9:32 AM on May 1, 2013: none

fatal: '1f52db4fdd3641e2c0ebbc2355ccc27547eaded7' does not point to a commit I updated this 2 times in a row, guess pulltester was too slow ^^.

BitcoinPullTester commented at 10:08 AM on May 1, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f810bac189c3f12de043026257adda7e76f27adf for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

sipa commented at 7:10 PM on May 1, 2013: member

ACK

gavinandresen commented at 5:00 PM on May 7, 2013: contributor

The wallet passphrase bits of this are unnecessary after #2625

Diapolo commented at 8:53 PM on May 7, 2013: none

@gavinandresen I will update after your patch was merged or you just merge this an rebase your pull :). That way the other changes in this pull can get in now.

As #2625 seems a little controversial, what is your oppinion on this now Gavin?

laanwj commented at 4:37 PM on May 30, 2013: member

Needs rebase as #2625 was merged

BitcoinPullTester commented at 11:05 PM on June 2, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/0ab7a2d0f0adb7020b0a75fb3a2426f29e61b943 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

changes to thread code (directly use boost::thread)

- removes our NewThread() function an replaces remaining calls with
  boost::thread with our TraceThread template
- remove ExitThread() function
- fix THREAD_PRIORITY_ABOVE_NORMAL for non Windows OSes

53e71135de

jgarzik referenced this in commit d1020b780a on Jun 10, 2013

jgarzik merged this on Jun 10, 2013

jgarzik closed this on Jun 10, 2013

DrahtBot locked this on Sep 8, 2021