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

Remove dead, buggy code #870

Merged
merged 1 commit into from Jul 9, 2021
Merged

Remove dead, buggy code #870

merged 1 commit into from Jul 9, 2021

Conversation

bluetarpmedia
Copy link
Contributor

@bluetarpmedia bluetarpmedia commented Jun 28, 2021

Replaced the condition checking whether this.job exists with an assert (like is used elsewhere), and removed the #pragma that disabled the CS8602 warning.

My reasoning is that SingleExecuteProtector can only ever be constructed with a non-null job (validates with Requires.NotNull) and the job is only set to null when TryExecute succeeds. Therefore if TryExecute reaches line 1230 then the job will exist.

Closes #544

… exists with an assert instead. SingleExecuteProtector can only ever be constructed with a non-null job (validates with Requires.NotNull) and the job is only set to null after TryExecute succeeds. Therefore the job will exist if TryExecute has an invokeDelegate (i.e. has not yet executed). Removed the #pragma that disabled the CS8602 warning.
Copy link
Member

@AArnott AArnott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll buy your reasoning. Thank you.

@AArnott AArnott changed the title Fix for #544 Remove dead, buggy code Jul 9, 2021
@AArnott AArnott added this to the v17.0 milestone Jul 9, 2021
@AArnott AArnott merged commit b0f7170 into microsoft:main Jul 9, 2021
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.

JTF.TryExecute may throw NullReferenceException
2 participants