Add a function to de-initialize the environment-variable state.#237
Add a function to de-initialize the environment-variable state.#237sunfishcode merged 2 commits intomainfrom
Conversation
Add a `__wasilibc_deinit_environ` function which clears the current environment variable state to the state where next time the environment variable functions are called, they'll reinitialize the environment. And add a `__wasilibc_maybe_reinitialize_environ_eagerly` function to reinitialize the environment variable state if `environ` or `_environ` are needed. These functions are needed by wizer to be able to suspend and resume a program and have it read new environment variables from the host environment; see bytecodealliance/wizer#8 for background.
Add a `__wasilibc_get_environ` function which returns the value of `environ` but without performing eager init.
d86f1f7 to
382d5cc
Compare
|
Is this PR waiting on anything? |
|
Just that we don't have a review approval yet. Anyone want to review this? |
| __wasi_sock_shutdown | ||
| __wasilibc_access | ||
| __wasilibc_cwd | ||
| __wasilibc_deinitialize_environ |
There was a problem hiding this comment.
Why bother with the laziness at all? i.e. why not just call this __wasilibc_reinitialize_environ?
There was a problem hiding this comment.
Another way of putting it: seems that any program that calls __wasilibc_deinitialize_environ would already be explicitly opting into using environment variables wouldn't it? So why delay re-initialization in that case, especially when it adds more complexity to our already pretty complex weak symbol dance.
There was a problem hiding this comment.
In the wizer use case, we want to de-initialize the environment before serializing the program state, and re-initialize it only after the program is deserialized and resumed. And, in the resumption case, to speed up startup time in the case that the program doesn't use any environment variables right away, it's nice for the re-initialization to be lazy.
There was a problem hiding this comment.
This seems rather specific to the wizer use case. I'm not particularly worried about this one minor change, but in general it seems a little backwards to have contort libc for specific use case like this.
IIUC the suspending and resuming thing that wizer is doing is supposed to be mostly transparent, so I would hope that there were not to many more changes like this needed, especially in libc.
The approach in general seem rather error prone since a program could read an environment variable during its startup phase than would end be our-of-date when the snapshot was loaded.. I guess those are just the fundamental risks of using such snapshotting?
There was a problem hiding this comment.
Yes, it is specific to the wizer use case. And yes, I expect this is just a fundamental risk of using such snapshotting.
I do expect this is fairly limited in scope. Libc doesn't cache much state, and most of what it does have, such as the malloc heap, the atexit list, errno, the strtok buffer, stdin/stdout/stderr, etc., isn't really desirable to reset. The one thing I can think of that we may want to be able to reset is the preopen table.
Add a
__wasilibc_deinit_environfunction which clears the currentenvironment variable state to the state where next time the environment
variable functions are called, they'll reinitialize the environment.
And add a
__wasilibc_maybe_reinitialize_environ_eagerlyfunction toreinitialize the environment variable state if
environor_environare needed.
These functions are needed by wizer to be able to suspend and resume
a program and have it read new environment variables from the host
environment; see bytecodealliance/wizer#8 for background.