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] Always allocate when computing temp path #81085

Merged
merged 3 commits into from
Jan 26, 2023

Conversation

maraf
Copy link
Member

@maraf maraf commented Jan 24, 2023

The wasm implementation of __randname uses pointer address as another entry into the randomness.

https:/emscripten-core/emscripten/blob/085fe968d43c7d3674376f29667d6e5f42b24966/system/lib/libc/musl/src/temp/__randname.c#L13

We may get the same random name if the same pointer passed twice before the clock ticks. The issue in the upstream repository is emscripten-core/emscripten#18591.

We have decided to take the simplest solution at the moment and consider a better one as part of the WASI file system implementation #81210

Fixes #73721

@maraf maraf added arch-wasm WebAssembly architecture area-System.IO labels Jan 24, 2023
@maraf maraf added this to the 8.0.0 milestone Jan 24, 2023
@maraf maraf self-assigned this Jan 24, 2023
@ghost
Copy link

ghost commented Jan 24, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Closes #73721

Author: maraf
Assignees: maraf
Labels:

arch-wasm, area-System.IO

Milestone: 8.0.0

@ghost
Copy link

ghost commented Jan 24, 2023

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Closes #73721

Author: maraf
Assignees: maraf
Labels:

arch-wasm, area-System.IO

Milestone: 8.0.0

@maraf
Copy link
Member Author

maraf commented Jan 24, 2023

/azp run runtime-wasm-libtests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lewing
Copy link
Member

lewing commented Jan 25, 2023

@maraf please reenable the tests that were disabled in #73834 as well

@maraf
Copy link
Member Author

maraf commented Jan 25, 2023

/azp run runtime-wasm-libtests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf maraf marked this pull request as ready for review January 26, 2023 10:33
@maraf
Copy link
Member Author

maraf commented Jan 26, 2023

/azp run runtime-wasm-libtests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maraf maraf merged commit 79e1abf into dotnet:main Jan 26, 2023
@maraf maraf deleted the WasmTempFileName branch January 26, 2023 14:31
@maraf
Copy link
Member Author

maraf commented Jan 26, 2023

/backport to release/7.0

@github-actions
Copy link
Contributor

Started backporting to release/7.0: https:/dotnet/runtime/actions/runs/4016125533

@ghost ghost locked as resolved and limited conversation to collaborators Feb 25, 2023
@pavelsavara
Copy link
Member

Emscripten landed fix for it emscripten-core/emscripten#18914 perhaps we could revert this change after we receive the new emscripten version

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.IO
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Path.GetTempFileName() sometimes fails on WASM due to insufficient randomness
6 participants