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

Recent version of mkpsxiso core dumps when creating certain ISOs #36

Closed
marco-calautti opened this issue Oct 11, 2022 · 4 comments
Closed

Comments

@marco-calautti
Copy link
Contributor

I extracted the bin of Soul Blade (USA) v1.1 with dumpsxiso, and then tried to rebuild the iso with mkpsxiso, but when adding the big XA file, the program throws a std::system_error with message what(): Resource temporarily unavailable.

Previous versions, such as the ones I build from my fork: https://github.com/marco-calautti/mkpsxiso
rebuild the bin without issues.

The recent rewrite has added a lot of changed code that I fear has not been properly tested, and thus regressed the tool, rather than improving it. I have encountered similar problems with other games, which now I cannot remember, while with previous versions everything was fine.

@marco-calautti
Copy link
Contributor Author

marco-calautti commented Oct 13, 2022

By having a quick look at the code, I believe the problem lies in the fact that mkpsxiso launches a thread (std::launch::async), for each sector for the XA file, to generate edc/ecc data, for each sector, which for very big files this would exhaust the maximum number of thread that can be run. In fact, when running mkpsxiso with gdb, I can see a lot of threads spawn.

I do not see the point of using threading for such a very simple task like generating ecc/edc, considering psx bins are not going to be very large anyway.

-EDIT- in fact, by removing all the async non-sense, everything works fine as expected. If using threads is really important, then I would suggest limiting the amount of tasks you spawn. My laptop has 32GB RAM, and this was not enough to pack a 434 MB XA file.

@marco-calautti
Copy link
Contributor Author

Probably you did not spot this issue, because the std library uses a thread pool in Windows. But on Linux, each std::async actually spawns a new thread. See https://stackoverflow.com/questions/29622179/async-uses-a-threadpool

@CookiePLMonster
Copy link
Collaborator

The recent rewrite has added a lot of changed code that I fear has not been properly tested, and thus regressed the tool, rather than improving it.

The previous version was not even capable of generating proper checksums so any and all files built through it were technically broken. I'm sorry that a regression crept in, but a rewrite was available for testing for a while before it got merged to the mainline repository and all reported issues were fixed. I'm sure you realize you can't fix bugs nobody spots, and it's not realistic to achieve 100% coverage of all possible discs.

Probably you did not spot this issue, because the std library uses a thread pool in Windows. But on Linux, each std::async actually spawns a new thread

That's quite disappointing. In this case I think the idea of using a thread pool as in the PR is fine.

@CookiePLMonster
Copy link
Collaborator

CookiePLMonster commented Mar 22, 2023

That's long fixed by #37.

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

No branches or pull requests

2 participants