Call sync.h primitives "locks" and "mutexes" instead of "blocks" and "waitable critical sections" to match current coding conventions and c++11 standard names.
This PR does not rename the "CCriticalSection" class (though this could be done as a followup) because it's used everywhere and would swamp the other changes in this PR. Plain mutexes should mostly be preferred instead of recursive mutexes in new code anyway.
Concept ACK.
Drop C prefix too?
Concept ACK.
Drop C prefix too?
Thanks, will wait for more feedback. I didn't rename CCriticalSection because it's used everywhere and changing it would make the diff 3 times as big (22 lines -> 73 lines). Also I think we will probably lean toward using non-recursive rather than recursive locks in new code, so CCriticalSection name should not matter as much going forward. I kept the C in CCriticalLock to be consistent with CCriticalSection. CCriticalLock is only used internally in sync.h so again it shouldn't have an impact on new code, and I didn't see a reason to break consistency within sync.h.
utACK eec3e2261eb895b704c5153203e6b1b946213667
To me, calling it just "Mutex"/"Lock" implies this is what people should use for new mutexs/locks, but that isn't what we want because it doesn't support DEBUG_LOCKORDER. Indeed, we maybe could make it support DEBUG_LOCKORDER and then start migrating to it over our current recursive stuff, but for now I'd prefer to make it more clear that the new typedefs should be discouraged.
Is DEBUG_LOCKORDER the only reason you want to discourage these?
I suppose, I mean as long as its clear that its non-recursive and someone doesn't introduce a bug on accident cause they're not paying attention, DEBUG_LOCKORDER would be my only complaint.
I'll just implement DEBUG_LOCKORDER for these. It should be pretty easy.
If we're renaming them anyway, why not call them what they are? RecursiveMutex. RecursiveLock, Mutex, Lock?
Do we need any special, project-local names for locking constructs at all or could we do with simply the c++11 naming? (I've wondered about this many times)
I'm all for using non-recursive locks in new code, using recursive locks is usually discouraged nowadays.
Indeed, after #11640, and because this is scripted, I'd be more than happy to see us drop the "CriticalSection" naming - no one uses that anymore...
Agree with @laanwj – let's use the standard C++11 naming instead of project-local names for the locking constructs.
Are there any good arguments for continuing with project-local names for the locking constructs?
<!--5d09a71f8925f3f132321140b44b946d-->The last travis run for this pull request was 87 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.
Please get rid of the "Merge remote-tracking branch 'origin/pull/11640/head'" merge commit
utACK otherwise
Call sync.h primitives "locks" and "mutexes" instead of "blocks" and "waitable
critical sections" to match current coding conventions and c++11 standard
names.
This PR does not rename the "CCriticalSection" class (though this could be done
as a followup) because it is used everywhere and would swamp the other changes
in this PR. Plain mutexes should mostly be preferred instead of recursive
mutexes in new code anyway.
-BEGIN VERIFY SCRIPT-
set -x
set -e
ren() { git grep -l $1 | xargs sed -i s/$1/$2/; }
ren CCriticalBlock UniqueLock
ren CWaitableCriticalSection Mutex
ren CConditionVariable std::condition_variable
ren cs_GenesisWait g_genesis_wait_mutex
ren condvar_GenesisWait g_genesis_wait_cv
perl -0777 -pi -e 's/.*typedef.*condition_variable.*\n\n?//g' src/sync.h
-END VERIFY SCRIPT-
Needs rebase
Needs rebase
Rebased f21c529c0b77f745f5ae2fd999b268d0d49f68a8 -> 190bf62be1214b072513c7fd7e01cc191723967c (pr/locksren.8 -> pr/locksren.9) after #11599 merge.
utACK 190bf62be1214b072513c7fd7e01cc191723967c