Make finalizer warning for shredded SecretBuffer async

[c42f]

This avoids IO in the finalizer where it's invalid to task switch.

Fixes #33696 (CC @staticfloat) by following the advice from #25141.

Amusingly enough, this seems to be a case where the unstructured "fire and forget" nature of @async is useful, at least from an implementation perspective (not really a counter example to #33248 but at least an interesting edge case to consider)

[c42f]

Oh, looks like we test for this warning which means that the test is now broken.

Furthermore, the unstructured async nature of this change makes such a test very hard to write, because there's no way to get a handle to the async task which produce the warning, in order to wait on it.

So maybe this approach is not going to work out. It's certainly a workaround and a general fix for logging from finalizers would be preferable.

Thoughts?

[staticfloat]

So the problematic test is

julia/test/secretbuffer.jl

Line 31 in 9ce24fd

@test_logs (:warn, r".*SecretBuffer was `shred!`ed by the GC.*") finalize(secret_b)

I wonder if we can do something like:

@test_logs ... begin
	finalize(secret_b)
	for idx in 1:1000
		yield()
	end
end

Clearly that's sub-optimal as it's not "guaranteed", but it may be enough to guarantee that in practice, that scheduled task will get executed. It would be nice to have a yield_everyone() that guaranteed that the scheduler allowed every other task to execute at least once before returning.

[c42f]

Yes that does seem like a viable workaround locally though it feels rather fragile.

Anyway, I've pushed it and we'll see what CI says on the buildbots.

[c42f]

In general the weird thing about running finalizers is that they don't naturally "belong" to any particular task. So errors logged from these should probably go to a special logger rather than the current task logger which "just happens" to be doing the GC.

[tkf]

Amusingly enough, this seems to be a case where the unstructured "fire and forget" nature of @async is useful, at least from an implementation perspective (not really a counter example to #33248 but at least an interesting edge case to consider)

Isn't this just a design flaw of Base.SecretBuffer API? Why not just use the do-block based approach so that you can lexically limit the lifetime of the security? But I've never used Base.SecretBuffer so I may be missing something important.

[c42f]

Why not just use the do-block based approach so that you can lexically limit the lifetime of the security

It would make a lot of sense to have a do block form for higher level operations such as Base.getpass() which currently return a SecretBuffer. The main use of this stuff in stdlib seems to be LibGit2.jl. Looking at the uses of shred! in there, it's not immediately clear how the secret data flows.

[c42f]

How about we just merge this? Despite it being an icky hack at least it works... which is more than can be said for the current situation.