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

fix setenv and getenv (especially on os x) #1333

Closed
boggle opened this issue Dec 19, 2011 · 10 comments
Closed

fix setenv and getenv (especially on os x) #1333

boggle opened this issue Dec 19, 2011 · 10 comments
Assignees
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows
Milestone

Comments

@boggle
Copy link
Contributor

boggle commented Dec 19, 2011

I digged a bit into setenv/getenv not always working on os x and found two issues:

  • Sadly the man page says:

    Successive calls to setenv() or putenv() assigning a differently sized
    value to the same name will result in a memory leak.  The FreeBSD seman-
    tics for these functions (namely, that the contents of value are copied
    and that old values remain accessible indefinitely) make this bug
    unavoidable.  Future versions may eliminate one or both of these semantic
    guarantees in order to fix the bug.
    

    maybe this can be patched up either by an additional unsetenv call /or/
    by setting a maximum buffer value when unset/size checking after having been set from rust.

  • setenv/getenv are not safe for concurrent use (acording to IEEE Std 1003.1, 2004 Edition) and thus
    should be wrapped by a task that processes set/get commands and is started on demand /or/ protected
    by a lock in the runtime. opinions on the correct approach wanted.

@boggle
Copy link
Contributor Author

boggle commented Dec 19, 2011

Summarizing the discussion around this on irc:

  • likely move setenv/getenv into rt and protect with a lock
  • there may be the need for a more genaral mechanism; if this shows up more often, the idea would
    be to add "native serial mods". All calls into such a mod would be protected by a single lock. Alternatively, have
    "singleton tasks" somehow. Deciding this needs time, so this issue is put on hold for now.
  • still not clear what is the correct way to deal with memory leakage of setenv/getenv accross platforms (esp mac/bsd)

@brson
Copy link
Contributor

brson commented Dec 19, 2011

Eventually we're going to need some sort of singleton tasks to hold a default IO loop

@brson
Copy link
Contributor

brson commented Dec 19, 2011

See also #884

@graydon
Copy link
Contributor

graydon commented Dec 19, 2011

Ugh, how ghastly.

I think the way I'd prefer to do this is to complete #553 -- add support for global variables as "unsafe elements of a module" -- and then add some utility functions to the runtime that (say) take a pointer-to-a-global and either atomically install a mutex in it or return a mutex already present in it. Then build a resource type that encapsulates that and we can write global-lock-guarded code in rust.

(Or some similar atomic-op-on-a-global abstraction; suggestions welcome)

Not great but we'll need unsafe globals for a few other grotty parts of hoisting rt code up into rust (logging, say).

@nikomatsakis
Copy link
Contributor

Personally I am not too concerned about memory leaks in setenv/getenv. I imagine this isn't fixed yet because people rarely set environment variables more than once or twice over the course of program execution.

@kud1ing
Copy link

kud1ing commented Jan 2, 2012

Probably see also #878

@catamorphism
Copy link
Contributor

Still blocked on #553.

@tedhorst
Copy link
Contributor

I actually got an double free error at one point when running the time::tests. I saved the report here:

https://gist.github.com/2403771

@ghost ghost assigned brson Apr 17, 2012
@brson
Copy link
Contributor

brson commented Apr 17, 2012

I'm going to put this on my list for 0.3 but may not get to it for a while. If somebody else wants to fix it then be my guest.

@brson
Copy link
Contributor

brson commented May 1, 2012

Fixed by 46cc11e

@brson brson closed this as completed May 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows
Projects
None yet
Development

No branches or pull requests

7 participants