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.

Sign up for GitHub

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

Jump to bottom

Fix crash with mod icon caching #1772

Merged
merged 6 commits into from Nov 6, 2023
Merged

Fix crash with mod icon caching #1772

merged 6 commits into from Nov 6, 2023

Conversation

Trial97
Copy link
Member

@Trial97 Trial97 commented Nov 1, 2023

How to reproduce:

  • download multiple large instances( more mods better they can be the same)
  • view the mods page for each one of them
  • after closing all edit instances windows
  • repeat the second and third steps until the UI doesn't respond

Small note I was able to reproduce this with three instances with 800+ mods
Here is the link to the discord message that started this: https://discord.com/channels/1031648380885147709/1118165588200657006/1169052911200903288 if they ever do an issue on GitHub link it to this ticket

@Trial97
jsut a overflow protection
2349f29 
Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@Trial97 Trial97 added bug Something isn't working simple change changelog:fixed A PR that appears under "Fixed" in the changelog labels Nov 1, 2023
@Trial97 Trial97 added this to the 8.0 milestone Nov 1, 2023
@Trial97 Trial97 linked an issue Nov 2, 2023 that may be closed by this pull request
Int Overflow Issues Caused by Failure To Release Image #1774
Closed
1 task
@Scrumplex Scrumplex changed the title fix: int overflow in PixmapCache::markCacheMissByEviciton Fix crash with mod icon caching Nov 2, 2023
@Trial97
made sure that we do not relay for undefined behavior
220a1de 
Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
TayouVR
TayouVR approved these changes Nov 5, 2023
View reviewed changes
Copy link
Member

@TayouVR TayouVR left a comment •
edited

Choose a reason for hiding this comment

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

I guess I understand kodes concerns, but besides that; as long as the cast back to int works right this code looks good to me.

@Trial97
changed type form double long to long long
710a48f 
Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@Trial97
apply suggested changes
cc29121 
Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@Trial97
Increased the step to around 10MB
670e91c 
Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
Scrumplex
Scrumplex reviewed Nov 6, 2023
View reviewed changes
launcher/MTPixmapCache.h Show resolved Hide resolved
launcher/MTPixmapCache.h Outdated Show resolved Hide resolved
@Trial97
Fixed comparation warning
ca226d2 
Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@Trial97 Trial97 merged commit 3d529b0 into PrismLauncher:develop Nov 6, 2023
31 checks passed
@Trial97 Trial97 deleted the pixmap_crash branch November 6, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Scrumplex Scrumplex Scrumplex approved these changes

@TayouVR TayouVR TayouVR approved these changes

Assignees
No one assigned
Labels
bug Something isn't working changelog:fixed A PR that appears under "Fixed" in the changelog simple change
Projects
None yet
Milestone
8.0
Development

Successfully merging this pull request may close these issues.

Int Overflow Issues Caused by Failure To Release Image
3 participants
@Trial97 @Scrumplex @TayouVR