Skip to content

Prevented foobar crash when exiting during AllMusic fetching#8

Open
TT-ReBORN wants to merge 1 commit into
Wil-B:mainfrom
TT-ReBORN:AllMusic-fetching-fix
Open

Prevented foobar crash when exiting during AllMusic fetching#8
TT-ReBORN wants to merge 1 commit into
Wil-B:mainfrom
TT-ReBORN:AllMusic-fetching-fix

Conversation

@TT-ReBORN

Copy link
Copy Markdown

As title says, more info here:
https://hydrogenaud.io/index.php/topic,121047.msg1044579.html#msg1044579

@regorxxx, you can close your open PR as it is with this PR deprecated: #7

Co-authored-by: regorxxx <regorxxx@users.noreply.github.com>
@regorxxx

regorxxx commented Jun 5, 2024

Copy link
Copy Markdown
Contributor

Closed it.

@TT-ReBORN just to confirm, the current code handles only a single request at a time. Is that intended?
Just asking, as far as I see the previous one allowed to send multiple request when changing selection and processing them on the background via promises (correct me if I'm wrong, but at least that seems the flow).
Your PR is not only a fix but a change on behavior, since every send call aborts the previous request instead of pushing them into a stack and aborting the entire stack on the unload callback (which was my proposed solution).

@TT-ReBORN

Copy link
Copy Markdown
Author

Yes, it should process single requests and start fresh again.
Imo this is the cleanest way and performance wise ( client <-> server side ), we do not need to have multiple request
processed because we only fetch text ( which is instant ) and no images like last.fm does.
I do not see any side effects, I have tested it with Majestyk and we didn't encounter any issues.

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.

2 participants