Use Foundation.ProcessInfo for environment parsing

[GNMoseke]

Resolves #571

• Simply replaces use of environ with Foundation.ProcessInfo

[Joannis]

My only (slight) concern is that I believe this wasn't thread safe in 5.9

I don't know if it's still the case - or if it matters much at all. Would need to verify that

[GNMoseke]

Yeah I saw that in the issue - I can verify in 5.9, will draft this guy for now.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.68%. Comparing base (e78cde7) to head (94ae83f).
Report is 144 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #573      +/-   ##
==========================================
- Coverage   84.86%   83.68%   -1.19%     
==========================================
  Files          98       99       +1     
  Lines        5320     4406     -914     
==========================================
- Hits         4515     3687     -828     
+ Misses        805      719      -86     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[GNMoseke]

At least as of 5.9.1, the implementation of ProcessInfo.processInfo.environment is the same as what this PR replaces - so there should be no difference for 5.9

The underlying access is indeed a char** as @adam-fowler commented in the original issue - I am sorry to say that my low-level cpp knowledge isn't super strong, so this appears thread safe to me, but take that with a grain of salt

[Joannis]

Hmm, you're right. I just looked it up: environ is not thread safe. Happy to accept this change, but we'll need synchronisation it seems.

[Joannis]

Hmm, you're right. I just looked it up: environ is not thread safe. Happy to accept this change, but we'll need synchronisation it seems.

[adam-fowler]

Given this is replacing like for like and in Swift 6 processInfo.environment is thread safe I'm happy to accept this PR. We can discuss any synchronisation support for 5.x in another PR. Plus we should verify it is an issue.

I retract my approval as we have a failure in the tests

Test failure

[GNMoseke]

Alright fellas sorry it took me so long to get back to this - as mentioned in #574 there's some thread safety issues here.

I think what was happening is that ProcessInfo.processInfo, as a static variable, along with Environment.getEnvironment as a static method were doing something funky when all tests were run in a single process (from a normal swift test invocation). About 1/5 times there I could reproduce the failure, including on a 6.0.1 toolchain.

I could not reproduce the failure running swift test --parallel, which is what leads me to believe that this only pops up in the case where all the tests are being started from the same process.

As a band-aid I renamed the env var that testSetForAllEnvironments uses, which feels like a bit of a hacky solution but does resolve the test failure.

All of the above said, I am very new to the codebase here and could absolutely be misunderstanding what's happening, so if any of that^ is untrue, please call me on it.

TL;DR I don't think this is actually an issue in a real running hummingbird instance since the server is started as a single process and the environment is being properly mutated - just a race in tests.

[adam-fowler]

As a band-aid I renamed the env var that testSetForAllEnvironments uses, which feels like a bit of a hacky solution but does resolve the test failure.

I think this is fine. The tests should be treated as independent and not affect each other.