Add custom iterators to SafeUnorderedMap #98
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Custom exception base class * tests for DynamicException * finish template behavior * Test dynamic excpetion in prod code * Fix member init * Fix linter errors * add more test cases * test ethCall thrpw * remove duplicate test * Change runtime excpetions to DynamicExceptions
This solves a race condition by making sure that during stop() the P2P::ManagerBase's thread pool waits for all tasks to be flushed out before its Session objects are closed and the underlying networking component is destroyed.
Move thread pool cleanup to after all sessions are deleted, to make sure message production actually stops. This was causing heap-use-after-free during ManagerBase destructor in testcases.
P2P::Session on receiving a message, passes it to ManagerBase::handleMessage with a weak_ptr instead.
* Replace weak_ptr<Session> with NodeId in Manager* APIs * Fix SonarQube issue * Remove retrieving session object for logging errors * Harden P2P networking shutdown sequence - Rule out any possibility that ManagerBase::sessions_ will grow (or shrink) after ManagerBase::close_ is set to true, so that ManagerBase::stop() can close all registered sessions - On ManagerBase::stop(), join the Session thread pool after Server and ClientFactory are stopped (possibly more correct) * P2P::ManagerBase: - removed closed_; replaced by started_ (ManagerBase should be able to start() and stop() repeatedly and safely) - added stateMutex_ to synchronize start(), stop() and thread pool - start() creates threadPool_ - stop() destroys threadPool_ (waits for all tasks and joins all threads, guaranteeing that there is no session/socket message handling code active when it is time to e.g. destroy the underlying io_context) - threadPool_ is now isolated and not forwarded to other objects; to use the ManagerBase thread pool, other objects have to call a method on ManagerBase that will service the request, such as ManagerBase::asyncHandleMessage() state.cpp (test): - increased timeout from 5s to 120s in a couple of futures to avoid random, performance-related failures (logs to cout if they exceed 5s, which in any case is rare) * add this->
Make rdPoS an exclusive member class of State + parametrize some vars in Options
- Implement operator=() - Implement size() - Fix erase-insert-commit bug in commit() - Improve & fix operator=() testcase that was calling the copy constructor instead - Add erase-insert-commit regression test - Fix all testcase asserts that depended on the placeholder size() code
SafeUnorderedMap fixes
- new custom iterators keep all internals hidden - remove range-erase erase(first, last) method (use it = erase(it) in a loop instead) - remove all node_type methods (harder to implement now that internals are not exposed) - remove insert(T&&) template methods (don't instantiate) - add testcases for functionality tests and for template instantiation tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch hides the
std::unordered_mapiterators that were exposed, and replaces them with newSafeUnorderedMap::iteratorandSafeUnorderedMap::const_iterator.As part of the proposed changes, some
SafeUnorderedMapmethods had to be removed. We can re-implement them later without exposing internal details, if it turns out that we will be needing them.Added tests for the new features, and also some additional tests to make sure all of the insert/emplace/erase methods from the template type get instantiated at least once.