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

Changes to the environment are invisible to Sys.getenv #4499

Closed
vicuna opened this issue Feb 5, 2008 · 12 comments
Closed

Changes to the environment are invisible to Sys.getenv #4499

vicuna opened this issue Feb 5, 2008 · 12 comments

Comments

@vicuna
Copy link

vicuna commented Feb 5, 2008

Original bug ID: 4499
Reporter: @alainfrisch
Assigned to: @nojb
Status: resolved (set by @nojb on 2018-04-05T21:16:35Z)
Resolution: fixed
Priority: normal
Severity: minor
OS: Windows
Target version: later
Category: platform support (windows, cross-compilation, etc)
Related to: #6903

Bug description

Windows has functions like SetEnvironmentVariable/GetEnvironmentVariable to access the process environment. The MS C runtime library also has getenv/putenv functions, but they operate on a snapshot of the environment taken when the process started. putenv also push modifications to the real environment through SetEnvironmentVariable.

So, if we mix in a single process OCaml code and other native code that set environment variables directly (from .Net, we only have SetEnvironmentVariable, not putenv), these changes are not seen from OCaml, which uses getenv.

A fix would be to use the "native" win32 functions for OCaml (not only for implementing Sys.getenv, but also for all the calls to getenv in the runtime system; and some care is needed for execve).

(At LexiFi, we use a temporary solution: we have a function to "refresh" the local copy of the environment in the libc by fetching all the real environment variable and putenv'ing them.)

File attachments

@vicuna
Copy link
Author

vicuna commented Feb 19, 2008

Comment author: @damiendoligez

I could easily argue that this is a bug in the getenv of the MS C runtime library.

@vicuna
Copy link
Author

vicuna commented Jun 20, 2012

Comment author: @alainfrisch

I could easily argue that this is a bug in the getenv of the MS C runtime library.

Probably, but it's easier (for us) to work-around the problem on the OCaml side than to convince Microsoft to fix their C runtime. As long as the effort remains reasonable, I think it's good to provide a coherent semantics between all the supported OCaml ports, even if this requires to work around bugs created by third parties.

@vicuna
Copy link
Author

vicuna commented Sep 15, 2012

Comment author: @damiendoligez

The right solution is probably to define our own functions sys_getenv and sys_putenv once with system-dependent code, and then use them everywhere.

In fact, from your description it seems we only need to redefine getenv.

@vicuna
Copy link
Author

vicuna commented Jul 29, 2013

Comment author: @xavierleroy

Pushing this for later, as I don't see the issue fixed in time for 4.01.

@vicuna
Copy link
Author

vicuna commented Jun 15, 2015

Comment author: adrien

What would be the functions involved? From a very very quick glance at MSDN, I don't see links to similar functions.

@vicuna
Copy link
Author

vicuna commented Nov 20, 2015

Comment author: @xavierleroy

I don't see this issue actively worked on, so I'm pushing it for after 4.03.

In the meantime it would be good to check whether the issue is still there in more recent versions of the MSVC runtime library.

@vicuna
Copy link
Author

vicuna commented Jun 18, 2017

Comment author: @nojb

Just to report that the issue persists in the MSVC bundled with the latest Visual Studio 2017.

Surprisingly enough, the Unicode version of getenv (_wgetenv) does not seem to have this problem! So if we manage to get #1200 merged, we will also be fixing this issue.

I uploaded getenv_test.c that exhibits both points. To compile, simply do cl getenv_test.c in a Developer Command Prompt.

@vicuna
Copy link
Author

vicuna commented Sep 25, 2017

Comment author: @alainfrisch

Fixed as part of #1200.

@vicuna
Copy link
Author

vicuna commented Sep 25, 2017

Comment author: @nojb

Unfortunately, I was too hasty about this. It turns out that the snapshot of the environment is taken lazily at the first call to _wgetenv, but not updated after that, so the problem remains depending on the precise sequence of calls.

The solution is probably to just switch to the native Win32 API {Get,Set}EnvironmentVariable.

@vicuna
Copy link
Author

vicuna commented Sep 27, 2017

Comment author: @dra27

@nojb - it should be the case that _wenviron is also initialised at program startup now, that's part of the reason for using wmain.

FTR, here's the full link to the Microsoft docs on how _environ and _wenviron work: https://docs.microsoft.com/en-gb/cpp/c-runtime-library/environ-wenviron

@vicuna
Copy link
Author

vicuna commented Apr 5, 2018

Comment author: @gasche

Can this be marked as resolved now that

#1479

was merged?

@vicuna vicuna closed this as completed Apr 5, 2018
@vicuna
Copy link
Author

vicuna commented Apr 5, 2018

Comment author: @nojb

Yes, done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants