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: printing optional access crash on Windows #38976
Conversation
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.
I think I understand the crash now, the old code was basically doing this:
SetSettings(job_settings.Clone(), std::move(job_settings));
and the result would depend on the order of parameter evaluation: for environments that evaluate job_settings.Clone()
first, things work expected; but for environments that evaluate std::move(job_settings)
first, the cloned value would be null.
So this PR does correctly fix the crash, but there should be some comments added on why it works.
a24a740
to
2d297d1
Compare
2d297d1
to
0392d20
Compare
Release Notes Persisted
|
I was unable to backport this PR to "25-x-y" cleanly; |
I have automatically backported this PR to "26-x-y", please check out #39039 |
@codebytere has manually backported this PR to "25-x-y", please check out #39095 |
Description of Change
Closes #38975
Closes #38944
Refs CL:4412367 and CL:4409898
Fixes an issue where printing on Windows could trigger a
RAW: Bad optional access
crash. This happened as a result of cloningjob_settings
and callingstd::move
on it in the same function call, which interestingly works just fine on macOS but doesn't on Windows.Checklist
npm test
passesRelease Notes
Notes: Fixes an issue where printing on Windows could trigger a crash.