Currently only a tiny fraction of all LOCK(...):s are documented with information on why each lock is held.
In the absence of such comments it can sometimes be hard to see why a specific lock is held from merely reading the code. This risks leading to situations where redundant LOCK(...):s are kept by mistake or "just to be safe" after refactoring, etc.
This PR improves the situation by adding comments with information on why a specific lock is being held by adding a comment using the form:
LOCK(cs_lockName); // variableBeingGuardedByThisLock, FunctionRequiringHoldingThisLock(...)
…
variableBeingGuardedByThisLock += 1;
…
FunctionRequiringHoldingThisLock(someThing);
Before this patch there were 9 documented and 514 undocumented LOCK/LOCK2:s –
$ git grep -E "[^a-zA-Z_](LOCK|LOCK2)\(" -- "*.h" "*.cpp" | grep -v sync.h | grep "//" | wc -l
9
$ git grep -E "[^a-zA-Z_](LOCK|LOCK2)\(" -- "*.h" "*.cpp" | grep -v sync.h | grep -v "//" | wc -l
514
After this patch there are 405 documented and 118 undocumented LOCK/LOCK2:s –
$ git grep -E "[^a-zA-Z_](LOCK|LOCK2)\(" -- "*.h" "*.cpp" | grep -v sync.h | grep "//" | wc -l
405
$ git grep -E "[^a-zA-Z_](LOCK|LOCK2)\(" -- "*.h" "*.cpp" | grep -v sync.h | grep -v "//" | wc -l
118
Note that there are still some locks left that are undocumented. I need help identifying the variable that is ultimately being guarded by each of these locks (it could be that some of these locks are unnecessary - if so, let me know!).
Please help by browsing the remaining undocumented locks using ...
$ git grep -3 -n -E '^[^/#]*[^A-Z_](LOCK|LOCK2)\([^/]*$' -- "*.cpp" "*.h"
... and see if you can help bring clarity. Each undocumented guarded variable that can be identified helps a lot - just leave a comment on the missing lock/guarded variable pair you've found and I'll track down all instances of that combination and add the appropriate comments :-)
That will be of great help for this comment PR and also for the related thread-safety annotations PR that I'm working on (see #11226).
