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

made ImageLoaderTask.TryLoadFromMemoryCacheAsync null safe #1524

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thisisthekap
Copy link

@thisisthekap thisisthekap commented Jan 25, 2021

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Fixes #1523

⤵️ What is the current behavior?

NullReferenceException thrown, when ImageLoaderTask.MemoryCache has entries with null values.

🆕 What is the new behavior (if this is a feature change)?

No NRE anymore, just reporting it as cache miss.

💥 Does this PR introduce a breaking change?

No.

🐛 Recommendations for testing

None, as #1523 was reported by our production error reporting, and unfortunately no reproduction steps are available.

📝 Links to relevant issues/docs

None

🤔 Checklist before submitting

  • All projects build
  • Follows style guide lines
  • Relevant documentation was updated
  • Rebased onto current develop

@thisisthekap
Copy link
Author

@daniel-luberda The failing AppVeyor build seems to be unrelated to my change.

@thisisthekap
Copy link
Author

@daniel-luberda Is my change breaking the CI?

@thisisthekap
Copy link
Author

@thisisthekap
Copy link
Author

@daniel-luberda @molinch Can you provide any schedule when this PR is going to be processed?

@artemtishchenkobinwell
Copy link

Same problem

@thisisthekap
Copy link
Author

@artemtishchenkobinwell I provided a fix in this PR, but unfortunately, I do not know if or when this PR will get reviewed.

@SelyutinSergey
Copy link

Same problem

1 similar comment
@maukur
Copy link

maukur commented Mar 15, 2021

Same problem

@thisisthekap
Copy link
Author

@artemtishchenkobinwell @SelyutinSergey @maukur If you feel fearless, you might build your custom nuget with my fix included. Of course without any warranty.

@thisisthekap
Copy link
Author

@daniel-luberda @molinch Is this PR going to get reviewed, or should it be closed?

@thisisthekap
Copy link
Author

I'll keep that PR open for fellow developers who are stumbling upon the related issue. As this repository seems to be abandoned, there is no further action taken from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants