Improve documentation of required error checks in drivers and unchecked invariants

[kevinmehall]

I'm working on a Rust library wrapping SoapySDR, and would like to make my bindings statically prevent any misuse of the SoapySDR API that would lead to undefined behavior and race conditions.

There seem to be a lot of implied invariants that could use better documentation, both for library users, and to set requirements for driver authors. Looking through a few driver modules, they mostly leave these kinds of issues up to the original device libraries. It defeats the purpose of a device-agnostic API if I have to read through the driver modules and the underlying libraries they use to determine what will or won't segfault. 😅

• What combinations are allowed when creating multiple streams on the same device simultaneously? I assume you can at least create one Tx stream and one Rx stream for full-duplex devices. Is it allowed to create multiple streams of the same direction if the channels don't overlap? Should driver modules return an error code when channels are in use by another stream (or other unsupported configurations), or will they crash?

• Reading and writing to a stream takes a single numElems argument to set the array size for all channels. However, the configuration of sample rate is per-channel. Are channels within the same stream allowed to have differing sample rates? (I'm wondering if in my Rust API, the sample rate configuration should be associated with streams rather than channels...)

• Are any Device methods forbidden while streaming is active? Will they return an error code if not, or will they crash?

• The only mention of thread-safety in the documentation is that Device::unmake is not thread-safe. However, its implementation is guarded by a mutex, so I'm not sure that's accurate.

• Are drivers required to make all non-stream Device methods thread safe? I'll assume that Streams cannot be shared safely across threads.

Happy to help write docs and/or add error-checking to drivers.

[guruofquality]

What combinations are allowed when creating multiple streams on the same device simultaneously? I assume you can at least create one Tx stream and one Rx stream for full-duplex devices. Is it allowed to create multiple streams of the same direction if the channels don't overlap? Should driver modules return an error code when channels are in use by another stream (or other unsupported configurations), or will they crash?

Any combination of tx and rx streams should be allowed with any number of simultaneous streams in the same direction. Typically multiple channels are only used when there is some expectation of alignment and common sample rate between the group of channels.

This is supported in SoapyRemote, but when you get down to device specifics, most of the dual channel devices can really only support streaming from one arbitrary channel, both, but not two separate streams of the same direction due to FPGA limitations.

The setupStream call should throw when an incompatible combination is requested. Most do this, but some do not check if the channel is already claimed by another open stream when they should. If you catch any implementations that violate this, lets open issues on them. I think the usb ones prevent this at the libusb driver/claim interface level. But I know that some dont, or we could get better error messages out of them.

Reading and writing to a stream takes a single numElems argument to set the array size for all channels. However, the configuration of sample rate is per-channel. Are channels within the same stream allowed to have differing sample rates? (I'm wondering if in my Rust API, the sample rate configuration should be associated with streams rather than channels...)

Typically, a stream of multiple channels, should have the same sample rate applied to all channels (homogeneous channels). Where as heterogeneous channels should be used only as separate streams. So although the API does not enforce it, there should be one sample rate per stream.

Many of the dual channel/MIMO devices will enforce this by merit of the hardware/fpga capabilities. You set the sample rate or frequency on one channel, you get it on both channels regardless.

I should note that the streaming API can support packets of data and in cases sample rates may not apply - implementation dependent FPGA processing IP for example. And that also, setting sample rate has value outside of the streaming itself.

Are any Device methods forbidden while streaming is active? Will they return an error code if not, or will they crash?

No. Most calls should ideally be configurable during streaming to make interactive apps and guis. This includes changing the sample rate.

The only mention of thread-safety in the documentation is that Device::unmake is not thread-safe. However, its implementation is guarded by a mutex, so I'm not sure that's accurate.

Device make/unmake and enumerate should all be thread safe now. Let me know if any current docs say otherwise. The library internal structures are protected by a mutex.

Are drivers required to make all non-stream Device methods thread safe? I'll assume that Streams cannot be shared safely across threads.

Thats correct, a stream should not be accessed by more than one thread.

For the device methods, the intention was that the device methods should only be accessed by a single thread -- which is often a single threaded client, gui thread, or actor thread. And while I think that this is the case, most implementations have been robustified anyway - with a mutex lock in each call, or a mutex around control transactions for usb or network. Some protect the set* call but save a cached value for the get* call which is inherently thread safe. Sometimes the underlying driver is threadsafe and protects all of the structures for your... so a lot of options and angles of attack here.

I can't name a specific application that is violating this, but to be robust, device implementations should have some kind of thread protection. Especially ones that are intended to be for the wide-world of SDR applications and not something very domain specific.

Happy to help write docs and/or add error-checking to drivers.

I would appreciate any such help. There is a driver guide that would benefit from a section on threading: https://github.com/pothosware/SoapySDR/wiki/DriverGuide

And ideally things should not crash, they should throw helpful error messages. So if you do see any crashing, open a bug and or PR on the relevant project.

Hope that helps! :-)

[kevinmehall]

Thanks! I opened a PR with some edits to the documentation based on this information.

For the device methods, the intention was that the device methods should only be accessed by a single thread -- which is often a single threaded client, gui thread, or actor thread. And while I think that this is the case, most implementations have been robustified anyway - with a mutex lock in each call, or a mutex around control transactions for usb or network.

I think documenting / requiring that device implementations make all configuration methods thread-safe is the right way to go. Especially with the way Device::make() can return the same Device object to multiple callers based on a string that is often user input, it's hard for a thread to be sure that it has exclusive access.

[kevinmehall]

All 4 of the drivers for which I have hardware to test (rtlsdr, hackrf, lime, and uhd) don't return an error when opening multiple simultaneous streams on the same channel (dev.setupStream(SOAPY_SDR_RX, "CF32", [0]) multiple times). rtlsdr instantly aborts when trying to readStream in such a configuration, while the other modules seem to be doing things like submitting the same libusb_transfer_t multiple times simultaneously and other fun data races.

[guruofquality]

old bug, closing. Any individual devices that need changes should get a PR.