Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Latent data race in WelchConfig #629

Open
palday opened this issue Mar 10, 2025 · 2 comments
Open

Latent data race in WelchConfig #629

palday opened this issue Mar 10, 2025 · 2 comments

Comments

@palday
Copy link

palday commented Mar 10, 2025

The WelchConfig docstring has an interesting nugget:

DSP.jl/src/periodograms.jl

Lines 545 to 549 in d08699a

!!! note
WelchConfig precomputes an fft plan, and preallocates the necessary intermediate buffers.
Thus, repeated calls to `welch_pgram` that use the same `WelchConfig` object
will be more efficient than otherwise possible.

The latter part encourages re-use, but the former part hints that concurrent re-use may have a race condition. It might be useful to call this out explicitly so that a naive

config = WelchConfig(...)
psds = Vector{Periodogram}(undef, length(signals))
Threads.@threads for (idx, sig) in enumerate(signals)
    psds[idx] = welch_pgram(sig, config)
end

doesn't become a user footgun

@wheeheee
Copy link
Member

Yeah, that's true for most of the configs. Maybe a warning is warranted but OTOH it should be obvious because it is stated that buffers are pre-allocated.

@wheeheee
Copy link
Member

Then again, that isn't really implicit in the common understanding of config. I guess this should be done...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants