Modify environment variables from native extensions (profilers etc)

[k15tfu]

@jkotas Hi again! I have a similar problem: The profiler modifies the environment variable at startup to control the way the child processes' profilers work. For example, it can be used to make profiling of child processes optional. Unfortunately coreclr does EnvironInitialize() before profiler startup, so I cannot unset environment variables. Can you suggest how to deal with that?

I see a few options:

1. call EnvironInitialize after profiler startup to give him a chance to modify them
2. allow to modify environment variables cache inside coreclr, for example by calling EnvironPutenv / SetEnvironmentVariableW

Originally posted by @k15tfu in https://github.com/dotnet/coreclr/issues/635#issuecomment-473985581

[k15tfu]

You can inject a call to Environment.SetEnvironmentVariable

Inject to what? Do you think the right solution is to instrument the user code to call Environment.SetEnvironmentVariable? It looks very difficult for anyone who just want to pass his variable to C#.

or make the profiler for child processes to be no-op

But it should be an option. How to control its behavior in this case?

[RussKeldorph]

@noahfalk

[jkotas]

@k15tfu I have listed the options that work today. This issue can be used to discuss a new profiler API to do this.

[noahfalk]

@k15tfu - What code is being used to launch the child processes? Is it code your profiler controls or it is outside of your control?

The typical way profilers avoid triggering for child processes or any other unintended process is that they test which process they reside within early in initialization. If it isn't a process that you want to load the profiler then you can safely return from DllCreateObject or any of the other early initialization APIs without doing any work. The profiler load will fail and the runtime will execute as normal with no profiling activity.

[k15tfu]

Hi! As I already said we want to control environment variables from profiler. On Windows all of these cases work:

• Run C# app with the profiler, and profile all its childs -- in this way we just set CORECLR_PROFILER & other variables before starting the process.
• Run C# app with the profiler, but do not profile its childs -- similar to no.1, but we clear these variables in ICorProfilerCallback::Initialize.
• Attach profiler to C# app, and profile its new childs -- attach to the app, and then set CORECLR_PROFILER & other variables in ICorProfilerCallback3::InitializeForAttach.

Moreover, we have some extra logic that depends on the position of the current process in the app's childs hierarchy, and its implementation is based on the inheritance of environment variables.

@noahfalk

What code is being used to launch the child processes? Is it code your profiler controls or it is outside of your control?

no, it's outside of our control.

The typical way profilers avoid triggering for child processes or any other unintended process is that they test which process they reside within early in initialization. If it isn't a process that you want to load the profiler then you can safely return from DllCreateObject or any of the other early initialization APIs without doing any work. The profiler load will fail and the runtime will execute as normal with no profiling activity.

It's okay if we don't want to profile childs always (no. 2), but how to support cases no.1 & no.3 ?

[ww898]

Hi @noahfalk, I follow this discussion for a long time and I have the idea. What do you think about extend the ICorProfilerInfo10 interface? Just add method to modify/delete environment variable in it. Let it works only in ICorProfilerCallback::Initialize() and ICorProfilerCallback3::InitializeForAttach().

[davmason]

@k15tfu @ww898 We can use this issue as a feature request to introduce a new profiler API to managed environment variables. It is certainly possible, but won't be in 3.0 since we are feature complete for 3.0. @jkotas and @noahfalk have given the only workarounds I know of already for 3.0.

[ww898]

@davmason Can you please return the SetEnvironmentVariableW export back till the feature will be implemented?

[k15tfu]

@davmason Yep, this API looks good to me. Is there a chance to get it implemented in 3.1?

Unfortunately these workarounds don't work in all scenarios: Make profiler for child processes to be no-op doesn't suite for cases no.1 & no.3. The other option about injecting the call will not work after attach.

[k15tfu]

@davmason @jkotas Is there anything I can help with to get it in 3.1?

[davmason]

@k15tfu no work has happened on this yet. For it to be considered for 3.1 the following steps would have to occur:

1. Define the spec for a new API and get signoff
2. Once the spec is signed off on , implement the API in master (5.0 branch)
   • Test the API before checkin
   • Check in to master for 5.0
3. Open a port request to 3.1
   • This would need signed off on by our management, I don't have control over that process

All of this would have to happen in a relatively short timeframe to make 3.1, and it would not be guaranteed to meet the 3.1 bar since there is a workaround and this is "feature work" (as opposed to bug fixes). There is also a strong chance that the work would go on too far to make the 3.1 cut off.

None of this work is currently scheduled so the likelihood that it will make 3.1 as it stands is very low (effectively it won't make 3.1).

If you feel strongly enough to help out, we are always happy to accept pull requests from the community.

[k15tfu]

@davmason @noahfalk @jkotas I added only one new method in corprof.idl: dotnet/coreclr#27157 . Anything else?

[davmason]

@k15tfu If we are adding profiler APIs to deal with environment variables, we should address the entire scenario. What if a profiler wants to retrieve the value environment variables or remove an environment variable? Removal could be done with the same SetEnvironmentVariable but providing null for the value, but we should also have a GetEnvironmentVariable

[k15tfu]

@davmason Sure, I'll add it too. Do we need something to get all of them (like GetEnvironmentStrings or C#'s GetEnvironmentVariables)?

[davmason]

@k15tfu I don't think we necessarily need it, but I also wouldn't be opposed to having it if it were relatively easy to implement. I suspect it's more trouble than it's worth since you'd have to follow the same pattern as the other profiler APIs that allow you to enumerate things (like EnumModules) and that's a lot of boilerplate code.

I think just Get/Set methods for one variable is all that's necessary.

[k15tfu]

Well, glad to close this issue. Thanks @davmason for your help.