-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 a heap-use-after-free in a unit test #7230
Fix a heap-use-after-free in a unit test #7230
Conversation
fdbserver/worker.actor.cpp
Outdated
state Standalone<StringRef> s = swVersionValue(swVersion); | ||
ErrorOr<Void> e = wait(errorOr(newVersionFile->write(s.begin(), s.size(), 0))); |
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.
Just a comment. We actually have holdWhile
and holdWhileVoid
actors to prevent object deconstruction.
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.
Yup, I think that would be an alternative to making s
a state variable. I don't have a preference. @sfc-gh-xwang are you saying you prefer holdWhile
to s
being a state variable? Happy to make that change if so
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.
If you do keep it a state variable, I would recommend renaming it. We generally try to discourage short names for state variables because of shadowing risks.
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.
Yes. I prefer holdWhile
. I just feel using state for a temp variables is somewhat too much, they don't need a lifetime as long as the outside actor.
Doxense CI Report for Windows 10
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for macOS BigSur 11.5.2
|
Doxense CI Report for Windows 10
|
The data passed to IAsyncFile::write must remain valid until the future is ready.
d6dfce1
to
edbb349
Compare
Doxense CI Report for Windows 10
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
The data passed to IAsyncFile::write must remain valid until the future
is ready.
Code-Reviewer Section
The general guidelines can be found here.
Please check each of the following things and check all boxes before accepting a PR.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
release-branch
ormain
if this is the youngest branch)