Bug 5390: Non-POD SquidConfig::ssl_client::sslContext exit crash

[eduard-bagdasaryan]

Squid may crash when the SquidConfig global is auto-destructed after
main() ends. Since SquidConfig global is used by cleanup code, we should
keep its fields alive, essentially emulating "No New Globals" policy
effects. This surgical fix will be followed up with more changes to
address general OpenSSL cleanup problems exposed by this bug.

This bug fix facilitates backporting by using FuturePeerContext shim.

[yadij]

It looks to me like there is something bigger going wrong. This smart-pointer still holding onto memory is just a symptom.

Ensuring that configFreeMemory() gets called during shutdown, somewhere after the Runners and before main() exits, would solve this along with quite a lot of other potential issues.

[eduard-bagdasaryan]

Ensuring that configFreeMemory() gets called during shutdown

I did not research whether we should do this (probably in a separate PR?) but this PR is about maintaining the existing SquidConfig invariant: keeping its members POD. The violation if this invariant caused the bug.

[rousskov]

Amos: It looks to me like there is something bigger going wrong.

Yes, there is. PR description explicitly talks about "general OpenSSL cleanup problems" that we will be addressing in followup PRs.

This smart-pointer still holding onto memory is just a symptom.

To be more precise, this smart pointer is not just a symptom: The pointer is a "SquidConfig should be a POD" rule violation (a problem this PR fixes), and it helped expose "general OpenSSL cleanup problems" (to be addressed in followup PRs).

Ensuring that configFreeMemory() gets called during shutdown, somewhere after the Runners and before main() exits, would solve this along with quite a lot of other potential issues.

I disagree that we should go back to that model: Long-term, configFreeMemory() should not exist at all. We are moving away from a flawed design where Squid code is supposed to work essentially without Config (or equivalent). INitial configuration and reconfiguration events may change Squid configuration, but that configuration should always be available/valid; freeing low-level pointers and resetting other low-level fields like current configFreeMemory() does causes more problems than it solves. We need to remove the last configFreeMemory() call, not restore another such call. More on that in 2022 commit 3a85184 message.

Eduard: this PR is about maintaining the existing SquidConfig invariant: keeping its members POD. The violation if this invariant caused the bug.

I agree that this minimal PR should be viewed that way, leaving other, more complex problems for other PRs to attack.