New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix locking issue with new mempool limiting #6889

Merged
merged 1 commit into from Oct 27, 2015

Conversation

Projects
None yet
4 participants
@jonasschnelli
Member

jonasschnelli commented Oct 26, 2015

Current master crashes on OSX with an exception: "boost: mutex lock failed in pthread_mutex_lock: Invalid argument".

mempool is a global object and gets initialized over cxx_global_var_init(). Calling LOCK() within constructor (of a object in global scope) is problematic (at least on OSX).

fix locking issue with new mempool limiting
Current master crashes on OSX with an exception: "boost: mutex lock failed in pthread_mutex_lock: Invalid argument"

@laanwj laanwj added the Bug label Oct 26, 2015

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Oct 26, 2015

Member

utACK

Member

laanwj commented Oct 26, 2015

utACK

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Oct 26, 2015

Member

The issue that this PR solves was introduced in #6722

Member

jonasschnelli commented Oct 26, 2015

The issue that this PR solves was introduced in #6722

@petertodd

This comment has been minimized.

Show comment
Hide comment
@petertodd

petertodd Oct 26, 2015

Contributor

utACK

Contributor

petertodd commented Oct 26, 2015

utACK

@@ -375,6 +375,7 @@ class CTxMemPool
void removeForBlock(const std::vector<CTransaction>& vtx, unsigned int nBlockHeight,
std::list<CTransaction>& conflicts, bool fCurrentEstimate = true);
void clear();

This comment has been minimized.

@dcousens

dcousens Oct 27, 2015

Contributor

nit: lockAndClear/clearSync maybe?

@dcousens

dcousens Oct 27, 2015

Contributor

nit: lockAndClear/clearSync maybe?

This comment has been minimized.

@laanwj

laanwj Oct 27, 2015

Member

Usually projects have some convention for this (eg _XX and XX, or XX_nolock and XX) . Where one is used internally (when lock already held) and the other externally (locks and calls inner function).
Preferable to have the names as similar as possible with just the lock/nonlock difference. lockAndClear and clearSync are too far apart to be easily recognizable as a set. So IMO the current naming is better.

@laanwj

laanwj Oct 27, 2015

Member

Usually projects have some convention for this (eg _XX and XX, or XX_nolock and XX) . Where one is used internally (when lock already held) and the other externally (locks and calls inner function).
Preferable to have the names as similar as possible with just the lock/nonlock difference. lockAndClear and clearSync are too far apart to be easily recognizable as a set. So IMO the current naming is better.

This comment has been minimized.

@dcousens

dcousens Oct 27, 2015

Contributor

👍 just not something I'd seen before.

@dcousens

dcousens Oct 27, 2015

Contributor

👍 just not something I'd seen before.

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Oct 27, 2015

Contributor

utACK

Contributor

dcousens commented Oct 27, 2015

utACK

@laanwj laanwj merged commit 0d699fc into bitcoin:master Oct 27, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Oct 27, 2015

Merge pull request #6889
0d699fc fix locking issue with new mempool limiting (Jonas Schnelli)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment