After I thoroughly studied C++/Boost thread sync support and its use in `simplewallet` I noticed that the `LOCK_IDLE_SCOPE` macro does not contain any outright bug, but is nevertheless needlessly complex and does things that have no effect, and I propose to simplify it from 10 lines of code down to 3.
Two threads within `simplewallet` must be in sync when accessing / changing the `wallet2` object: The main thread and the idle thread that does autorefresh. Sync is done through locks on the `m_idle_mutex` mutex: The thread that manages to put a lock on that mutex can be sure to have exclusive access to the wallet until the lock is lifted.
The main thread uses the `LOCK_IDLE_SCOPE` macro to lock and relies on the destructor of the `boost::unique_lock` object at the end of the method in question to unlock.
Locks on that mutex alone are sufficient already to sync the access of the two threads to the `wallet2` resource. It’s unnecessary, as was done until now, to temporarily set the `m_auto_refresh_enabled` option to `false` as a second, additional measure: The idle thread can’t do anything anyway if it does not currently hold a lock on the mutex.
It makes sense to call `m_idle_cond.notify_one` within `LOCK_IDLE_SCOPE`: You want to stop the up-to-90-seconds wait of the idle thread between auto refreshes and make it refresh right away after the wallet state changed. But there is no need to put a lock on the mutex and use a loop for doing so.
I am aware that syncing threads the wrong way can lead to subtle and rarely occuring bugs because of the dependence on timing, so I tested carefully. I made the refresh thread doing a refresh 3 times every second instead of once every 90 seconds attempting to provoke a sync breakdown, without success.
The `wallet` class that the GUI wallet uses to access its `wallet2` object also works with mutexes, but such a complicated logic like until now in `LOCK_IDLE_SCOPE` is nowhere in sight.
You can view, comment on, or merge this pull request online at:
— Commit Summary —
* simplewallet: Simplify LOCK_IDLE_SCOPE macro
— File Changes —
M src/simplewallet/simplewallet.cpp (13)
— Patch Links —