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

wasm-opt might run unnecessarily repetitively on its own output #1190

Open
quark-zju opened this issue Nov 11, 2022 · 2 comments · May be fixed by #1191
Open

wasm-opt might run unnecessarily repetitively on its own output #1190

quark-zju opened this issue Nov 11, 2022 · 2 comments · May be fixed by #1191

Comments

@quark-zju
Copy link

🐛 Bug description

On certain filesystems wasm-pack might repetitively call wasm-opt with previous output from wasm-opt as new input of wasm-opt and it's unspecified when the loop ends.

🤔 Expected Behavior

wasm-opt runs exactly once per input file.

👟 Steps to reproduce

This was discovered on EdenFS. But EdenFS can be tricky to build or config externally.

The problem seems obvious, though. In this loop, the directory is being readdir()-ed and written to at the same time. It's unspecified whether the newly written files will appear in the next iteration of readdir() or not. If it does appear, then wasm-opt runs again on its input. I have a fix at quark-zju@37650af.

See also https://stackoverflow.com/questions/39015527/is-it-safe-to-rename-files-while-using-readdir

🌍 Your environment

Include the relevant details of your environment.
wasm-pack version: 0.10.3
rustc version: 1.65.0

quark-zju added a commit to quark-zju/wasm-pack that referenced this issue Nov 11, 2022
POSIX says [1]:

> If a file is removed from or added to the directory after the most recent
> call to opendir() or rewinddir(), whether a subsequent call to readdir()
> returns an entry for that file is unspecified.

Writing `wasm-opt` output, renaming in a same directory being `readdir()`-ed is
unspecified, and can cause the `readdir()` to loop indefinitely on certain
filesystems.

Fix it by collecting `readdir()` result first before making changes to the
directory.

Resolves rustwasm#1190.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/readdir.html
@quark-zju quark-zju linked a pull request Nov 11, 2022 that will close this issue
3 tasks
@GuyLewin
Copy link

GuyLewin commented Aug 8, 2023

+1 - this happens to me as well on EdenFS.
Is there anything we can do to land this?

@dannymcgee
Copy link

Is this why our Netlify pipeline spends like 15-20 minutes on the wasm-opt step?

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 a pull request may close this issue.

3 participants