Skip to content

Add a function to de-initialize the environment-variable state.#237

Merged
sunfishcode merged 2 commits intomainfrom
sunfishcode/deinit-environ
May 25, 2021
Merged

Add a function to de-initialize the environment-variable state.#237
sunfishcode merged 2 commits intomainfrom
sunfishcode/deinit-environ

Conversation

@sunfishcode
Copy link
Copy Markdown
Member

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_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.
@sunfishcode sunfishcode force-pushed the sunfishcode/deinit-environ branch from d86f1f7 to 382d5cc Compare March 18, 2021 16:48
@fitzgen
Copy link
Copy Markdown

fitzgen commented May 19, 2021

Is this PR waiting on anything?

@sunfishcode
Copy link
Copy Markdown
Member Author

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
Copy link
Copy Markdown
Member

@sbc100 sbc100 May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bother with the laziness at all? i.e. why not just call this __wasilibc_reinitialize_environ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sunfishcode sunfishcode merged commit 3c4a3f9 into main May 25, 2021
@sunfishcode sunfishcode deleted the sunfishcode/deinit-environ branch May 25, 2021 22:25
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.

3 participants