feat: make Sapi work with ZTS builds

[Qard]

Description

This enables a SapiModule to work in ZTS mode. I'm using SapiModule directly in my case rather than going through the Embed type. I missed this when submitting my other PR to expand SapiBuilder.

Not sure what making Embed ZTS-capable would look like as it would require constructing a single SapiModule and reusing it so the API might need to be different or it might need to do some internal OnceCell magic.

Not entirely sure how to test this in a way that's not rather complicated. The only observable difference I'm aware of is that without the change it might randomly segfault eventually with enough parallel requests. Any ideas? 😅

I'm already using this code in my fork, so I know it works, just writing a test that's not gigantic would be a challenge. 🤔

Checklist

Check the boxes that apply (put an x in the brackets, like [x]). You can also check boxes after the PR is created.

• I have read the contribution guidelines.
• I have added tests that prove my code works as expected.
• I have added documentation if applicable.
• I have added a migration guide if applicable.

❤️ Thank you for your contribution!

[coveralls]

Pull Request Test Coverage Report for Build 16186430696

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 22.342%

---

Totals
Change from base Build 16098979450:	0.0%
Covered Lines:	870
Relevant Lines:	3894

---

💛 - Coveralls

[Xenira]

Not entirely sure how to test this in a way that's not rather complicated. The only observable difference I'm aware of is that without the change it might randomly segfault eventually with enough parallel requests. Any ideas? 😅

I'm already using this code in my fork, so I know it works, just writing a test that's not gigantic would be a challenge. 🤔

Not having tests here would be fine here. It is not a core functionality and the effort is not justified. Should this break at any time we might want to consider doing it anyways, but for now I am ok with not testing.

[Qard]

I actually was able to put together a very simple test for multi-threaded SAPI. I misremembered a bit about what was needed for Embed, so you can actually use that in a multi-threaded environment too. 🙂

[Qard]

Ah, the test seems to be a bit problematic. It runs fine with cargo test --test sapi_tests --features embed, but if you turn on the closure feature (which CI is doing) it explodes because apparently you can only call get_module once as it appears the Closure::build() initialization doesn't clean up after itself when the module is torn down.

Possibly best to just delete that test for now, unless you know a good way to get that to clean up properly? 🤔

[Xenira]

Ah, the test seems to be a bit problematic. It runs fine with cargo test --test sapi_tests --features embed, but if you turn on the closure feature (which CI is doing) it explodes because apparently you can only call get_module once as it appears the Closure::build() initialization doesn't clean up after itself when the module is torn down.

Possibly best to just delete that test for now, unless you know a good way to get that to clean up properly? 🤔

Could you extract the test case into a separate MR? That way we don't loose track of it and can check if there is a good way of cleaning up. Would be ok with merging this without the testcase being included.

[Qard]

Sure, I'll do that today.

[Qard]

The test has been moved to #496. I'll rebase that and continue after this lands.