iperf icon indicating copy to clipboard operation
iperf copied to clipboard

libiperf thread safety

Open jakemoroni opened this issue 1 year ago • 1 comments

It appears that libiperf may not be thread safe.

If I create a simple test program that starts both a server and client in the same process, but on different threads, there are some inconsistencies observed related to the once-per-second periodic log output time intervals. For example, some of the intervals have the same start and end time, resulting in bogus rate calculations. The final printout at the end of the test is always correct though.

After a little investigation, I think it could be related to the use of static globals in timer.c: https://github.com/esnet/iperf/blob/a9a55e409d07537c6951f15d30885755d3d3cf29/src/timer.c#L36

A potential solution could be moving these into struct iperf_test so that each test has its own complete context, but perhaps there is something I am overlooking.

Other than that, everything seems to work fine. There is also the issue of "i_errno" being shared, but that's to be expected.

jakemoroni avatar Oct 25 '23 14:10 jakemoroni

Thanks for the bug report! You've identified an issue, albeit one I don't think anybody had considered until now. There's a basic (but unwritten) assumption in libiperf (as it stands on master today) that it's only going to run in one thread in a process, so you can have things like timers that are global to the entire process.

Having multiple of these things would sorta help, but I'm not sure if doing this per iperf_test object is exactly right because the timers are supposed to enable a single thread to service multiple tests. If it were possible to do one-per-thread (?) that might give the right behavior, maybe.

When we merge the multi-threading code to the mainline (Real Soon Now) that'll change the way we use Timer objects. They'll only be used by the main thread, which will handle the control connection but not the worker threads that handle the individual tests. As a note for anyone reading the mt code, there's probably some cleanup we need to do in this area. I am not sure how that will affect the issue you're seeing (my complete guess is that it won't help but won't make it worse).

bmah888 avatar Oct 27 '23 16:10 bmah888