-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Proposed 2.2.0-b3 #4995
Merged
Merged
Proposed 2.2.0-b3 #4995
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
…4751) This amendment, `fixPreviousTxnID`, adds `PreviousTxnID` and `PreviousTxnLgrSequence` as fields to all ledger objects that did not already have them included (`DirectoryNode`, `Amendments`, `FeeSettings`, `NegativeUNL`, and `AMM`). This makes it much easier to go through the history of these ledger objects.
* Amend `.codecov.yml` to disable coverage reporting of test sources and explicitly set most parameters * Increase codecov upload retry time to 210s (from 35s) * Upgrade gcovr adding support for more coverage formats (lcov, clover, jacoco) * Upgrade github actions in coverage workflow * Explicitly disable codecov plugins (also removing `gcov` coverage, which is not correctly handled by codecov codecov/feedback#334)
--------- Co-authored-by: Howard Hinnant <howard.hinnant@gmail.com> Co-authored-by: Mark Travis <mtravis@ripple.com> Co-authored-by: Bronek Kozicki <brok@incorrekt.com> Co-authored-by: Mayukha Vadari <mvadari@gmail.com> Co-authored-by: Chenna Keshava <ckeshavabs@gmail.com>
…#4982) * Specifically, test using tfLPToken flag
The `rotateWithLock` function holds a lock while it calls a callback function that's passed in by the caller. This is a problematic design that needs to be used very carefully. In this case, at least one caller passed in a callback that eventually relocks the mutex on the same thread, causing UB (a deadlock was observed). The caller was from SHAMapStoreImpl, and it called `clearCaches`. This `clearCaches` can potentially call `fetchNodeObject`, which tried to relock the mutex. This patch resolves the issue by changing the mutex type to a `recursive_mutex`. Ideally, the code should be rewritten so it doesn't hold the mutex during the callback and the mutex should be changed back to a regular mutex. Co-authored-by: Ed Hennis <ed@ripple.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release #4995 +/- ##
==========================================
- Coverage 77.0% 71.0% -6.0%
==========================================
Files 1129 796 -333
Lines 131914 66727 -65187
Branches 39629 10971 -28658
==========================================
- Hits 101539 47348 -54191
+ Misses 24459 19379 -5080
+ Partials 5916 0 -5916
|
scottschurr
approved these changes
Apr 19, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 builds, unit tests, and syncs for me on macOS.
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.
High Level Overview of Change
This is a beta for the 2.2.0 release.
Highlights:
fixPreviousTxnID
: addsfPreviousTxnID
/sfPreviousTxnLgrSequence
to all ledger objects #4751The base branch is
release
. All releases (including betas) go inrelease
. This PR will be merged with--ff-only
(not squashed or rebased, and not using the GitHub UI) to bothrelease
anddevelop
.Context of Change
This introduces
fixPreviousTxnID
amendment to make it easier to analyze the history of all objects.It also fixes a deadlock situation during
online_delete
, improved code coverage reporting, and adds several tests to improve coverage of AMM functionality.Type of Change
API Impact
No API impact.