Comment 8 for bug 1327426

Revision history for this message
Michi Henning (michihenning) wrote :

I'm beginning to think that the entire idea of using environment variables for all this is problematic.

We had long discussions about whether or not it is necessary for the environment variables to be set before a scope's create function is called. The only way the scope could access these environment variables is either from within the implementation of the create function, or from the constructor of the ScopeBase that the create function returns. Neither of which is necessary, IMO, seeing that we have a separate start() method to initialise the scope. (I added this method specifically to provide some threading guarantees and to create a well-defined initialisation point.)

Second, it is conceivable that a scope has global object instances whose constructors might access an environment variable. Personally, I have absolutely no sympathy for someone who writes code like that. People who do this deserve what they get, IMO, and I feel no need whatsoever to provide scope-specific information at the time global constructors run.

Third, there is a more serious concern. I have been very careful to ensure that the scope run time has a well-defined life cycle. We control precisely when and how a run time instance is created and when it is destroyed. It is possible to have multiple run time instances active concurrently in the same process, and we actually do this in a number of places (and not just the tests). There are no global/static objects in the library that can break this isolation: run time instances are separate and independent, each with its own configuration, threads, etc.

The environment variables shoot a big hole through all this because environment variables are shared globally for everything in a process. In particular, running a second instance of the scopes run time causes it to smash the environment variable settings for the first instance. True, in a production environment, we run each scope in its own process, but the whole thing still feels wrong to me because it ties the run times together in unexpected ways. I had a surprising failure yesterday because of exactly this. We have at least one place in our code where we temporarily create a second run time instance. That second instance promptly overwrote the environment of the first one.

So, I want to move away from environment variables, except for LD_LIBRARY_PATH (and maybe PATH), which we can set easily enough before the scope's .so is loaded.

Instead of getting the scope to read environment variables, we can provide accessors on ScopeBase.

We already have:

scope_directory() // returns the install dir, corresponds to XDG_CONFIG_HOME
cache_directory() // returns the dir that is writable to the scope, corresponds to XDG_DATA_HOME

This leaves PATH, TMPDIR, XDG_RUNTIME_DIR, and UBUNTU_APPLICATION_ISOLATION.

XDG_RUNTIME_DIR and TMPDIR need to be the same anyway, because TMPDIR has to be a sub-directory of /run/user/<uid>, and the scope can't write to locations "above" that, so it doesn't make sense to set XDG_RUNTIME_DIR to anything but TMPDIR.

We can add

tmp_directory() // returns a writable tmp directory somewhere below /run/user/<uid>

This will take care of TMPDIR and XDG_RUNTIIME_DIR.

For PATH, I'm not sure I see the point. The scope knows where it is installed (in scope_directory()). If the scope wants to exec something in its own bin directory, it can just exec it without having to rely on PATH. We can set a defined path, no problem. But whatever we set it to will have to be a standard path that is the same for all scopes, not containing any scope-specific directories.

That leaves UBUNTU_APPLICATION_ISOLATION. I did a Google search and couldn't find any doc on that specific variable. But is it really needed? It is understood that scopes are confined. If a scope really needs to find out whether it is currently under some Apparmor profile, it can just try doing something that won't work if confined. If the attempt fails, the scope knows it's currently confined.