Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

valgrind: use function-local initialization to implement singleton #174

Merged
merged 6 commits into from
Oct 16, 2018

Conversation

neverchanje
Copy link
Contributor

@neverchanje neverchanje commented Oct 15, 2018

Valgrind warns that the internal pointer of singleton is not deleted (singleton::_instance), to fix this we rewrite the singleton implementation.

function-local initialization: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2660.htm#Local

See this if performance issue is your concern: https://stackoverflow.com/questions/23829389/does-a-function-local-static-variable-automatically-incur-a-branch?noredirect=1&lq=1

@neverchanje neverchanje added the area/memcheck PR or issues related to valgrind memcheck label Oct 15, 2018
@neverchanje neverchanje mentioned this pull request Oct 15, 2018
9 tasks
static std::atomic<int> _l;

private:
singleton(const singleton &);
Copy link
Contributor

@shengofsun shengofsun Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nocopyable的feature没了。毕竟这是个utility库,我们还是严谨些吧。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已改

qinzuoyan
qinzuoyan previously approved these changes Oct 16, 2018
* xxxx-xx-xx, author, first version
* xxxx-xx-xx, author, fix bug about xxx
*/

#pragma once

#include <mutex>
#include <atomic>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些include删了吧

@neverchanje neverchanje merged commit 2b19784 into XiaoMi:master Oct 16, 2018
vagetablechicken pushed a commit to vagetablechicken/rdsn that referenced this pull request Nov 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/memcheck PR or issues related to valgrind memcheck
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants