Skip to content
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

Misc. fixes for 6.x #991

Merged
merged 4 commits into from
Dec 26, 2019
Merged

Misc. fixes for 6.x #991

merged 4 commits into from
Dec 26, 2019

Conversation

jagerman
Copy link
Member

Fixes some compilation issues and a db txn bug.

There's no way to know which of this was fired (the one above, in the
same function, has identical wording).
Also don't return a completely useless bool.
Blockchain::prepare_handle_incoming_blocks locks m_tx_pool, but uses a
local RAII lock on the blockchain object itself, then also starts a
batch.  Blockchain::cleanup_handle_incoming_blocks then also takes out a
local RAII blockchain lock, then cleans up the batch.

But the lmdb layer is retarded in that it throws an exception if any
thread tries to write to the database while a batch is active in another
thread, and so the blockchain lock is *also* used as a guard writes.
Holding an open batch but *not* holding the blockchain lock then hits
this exception if a write arrives in another thread at just the right
time.

This is, of course, terrible design at multiple layers, but this close
to release I am reluctant to make more drastic fixes.

Other small changes here:

- All the locks in `blockchain.cpp` now use tools::unique_lock or
  tools::unique_locks rather than the nasty epee macro.  This also
  reduces the likelihood of accidental deadlock because this means the
  dual txpool-blockchain locks are not taken out simultaneously via
  std::lock rather than sequentially.

- Removed a completely useless "if (1)".  Git blame shows that there was
  previously a condition here, but apparently the upstream monero author
  who changed it was afraid of removing the `if` keyword.

- Reduced the sleep in the loop that waits for a batch to 100ms from
  1000ms because sleeping for a full second for a fairly light test is
  insane.

- boost isn't happy calling boost::lock() on the tx pool or blockchain
  object because the lock/unlock/try_lock methods are const, and so the
  workaround of using boost::lock because std::lock and
  std::shared_time_mutex are broken on the macOS SDK 10.11 that we use
  for mac builds now requires extra workarounds.  Joy.
@jagerman jagerman merged commit 6e436c2 into oxen-io:dev Dec 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant