Merge lp:~michihenning/unity-scopes-api/signal-before-unlock into lp:unity-scopes-api
Proposed by
Michi Henning
Status: | Merged |
---|---|
Approved by: | Michi Henning |
Approved revision: | 41 |
Merged at revision: | 41 |
Proposed branch: | lp:~michihenning/unity-scopes-api/signal-before-unlock |
Merge into: | lp:unity-scopes-api |
Diff against target: |
321 lines (+44/-77) 8 files modified
demo/client.cpp (+2/-4) demo/scope-C.cpp (+5/-10) demo/scope-D.cpp (+5/-10) src/unity/api/scopes/internal/Reaper.cpp (+5/-7) src/unity/api/scopes/internal/ReplyObject.cpp (+0/-1) src/unity/api/scopes/internal/ScopeLoader.cpp (+27/-41) src/unity/api/scopes/internal/zmq_middleware/ObjectAdapter.cpp (+0/-2) src/unity/api/scopes/internal/zmq_middleware/ZmqMiddleware.cpp (+0/-2) |
To merge this branch: | bzr merge lp:~michihenning/unity-scopes-api/signal-before-unlock |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Marcus Tomlinson (community) | Approve | ||
Review via email: mp+192634@code.launchpad.net |
Commit message
Changed code to signal condvar while still holding the lock. No correctness issue here. The change is mainly to stop warnings from helgrind, which doesn't like signals outside a lock.
Description of the change
Changed code to signal condvar while still holding the lock. No correctness issue here. The change is mainly to stop warnings from helgrind, which doesn't like signals outside a lock.
To post a comment you must log in.
I believe we should always attempt to have mutexes lock and unlock at their most efficient points in code (Its one of those optimisations you can foresee while coding). I see RAII mutexes as great safety nets when making a mistake, or when a scope is exited unexpectedly. My point is, perhaps we should not remove those lock.unlock() lines completely, but instead move them a line below the notify. This not only gives the thread a few more clock ticks now, but later allows for code to be added to these methods without causing the mutex to be held any longer than it needs to.