SDRPlay 3.08 ABI breakage #43
Comments
|
@gvanem - thanks for reporting the problem; since my main development platform is Linux, I haven't had the chance to test the new version of the API (I just checked the downloads section on SDRplay's web site, and for Linux they still have version 3.07). Anyway, I think that your scenario should have at least triggered this warning (https://github.com/pothosware/SoapySDRPlay3/blob/master/sdrplay_api.cpp#L49): If you don't mind please check your logs to see if that warning is there before Unfortunately I don't think that checking for the size of that structure at runtime is possible in C/C++, since the If you do find that warning above in your logs, please let me know, and tonight after work I'll make the change to the SoapySDRPlay3 driver to handle that as a fatal error instead. Franco |
So where does Soapy write log-files? The only thing I have from that old build is: The above is in the destructor. I think maybe the reason for the crash was that the SDRPlay_APIservice was not started. |
|
@gvanem - I see the message you posted shows that the error occurs in The errors/warnings should be displayed on the console/terminal when you run Franco |
|
As I mentioned this morning, I just created a new branch called Please give it a try when you have sometime and, if it solves your problem, I'll push this simple change into the Franco |
Tried it. I did this:
Seems not related to the API mismatch. But this: And yes, the service ( I'll try to build an unoptimized PS. here is the complete edited call-stack: crash.txt |
|
@gvanem - thanks for giving it a try. This evening I booted my PC into Windows 10 (I have a dual boot PC that most of the time I use under Linux), ran all the latest Window updates, and then ran the scenario that I think you have there:
As you can see, no problems here. I then downloaded the latest API for Windows from SDRplay (version 3.08), installed it on top of the previous one and ran
This is exactly the behavior I would expect to see in the scenario that you have there, where the version of the SDRplay API at compile time does not match the version of the SDRplay API at run time. Franco |
Yes, yes. I've no problem with that code. But here are my steps once more:
But the crash seems to occur in the static destructor of |
|
I think maybe the core issue is that the Edit: The old 3.7 |
|
@gvanem - thanks for the detailed analysis of this problem. Based on your conclusions, I think you should be able to observe the same behavior even when the As per using an exception to signal a fatal error in the SoapySDRPlay driver, we had a discussion about it in the past - see this comment: #16 (comment) and related comments in the same thread. Finally I am not sure what exactly we are trying to "fix" here: since having a different version of the SDRplay API at runtime is already a fatal unrecoverable error, which requires rebuilding the SoapySDRPlay3 module, once the module throws a runtime error, and the client program that invokes it exits/aborts, there's not much else that can be done IMHO. Franco |
Fix the horrid C++ code to avoid a crash? --- a/SoapySDRPlay.hpp 2021-06-30 14:13:02
+++ b/SoapySDRPlay.hpp 2021-08-28 15:31:42
@@ -356,6 +356,7 @@
private:
static float ver;
+ static bool valid;
sdrplay_api();
public:
--- a/sdrplay_api.cpp 2020-12-31 17:47:40
+++ b/sdrplay_api.cpp 2021-08-28 15:31:07
@@ -25,36 +25,43 @@
#include "SoapySDRPlay.hpp"
float SoapySDRPlay::sdrplay_api::ver = 0.0;
+bool SoapySDRPlay::sdrplay_api::valid = false;
// Singleton class for SDRplay API (only one per process)
SoapySDRPlay::sdrplay_api::sdrplay_api()
{
sdrplay_api_ErrT err;
+
// Open API
err = sdrplay_api_Open();
if (err != sdrplay_api_Success) {
SoapySDR_logf(SOAPY_SDR_ERROR, "sdrplay_api_Open() Error: %s", sdrplay_api_GetErrorString(err));
SoapySDR_logf(SOAPY_SDR_ERROR, "Please check the sdrplay_api service to make sure it is up. If it is up, please restart it.");
- throw std::runtime_error("sdrplay_api_Open() failed");
+ return;
}
// Check API versions match
err = sdrplay_api_ApiVersion(&ver);
if (err != sdrplay_api_Success) {
- SoapySDR_logf(SOAPY_SDR_ERROR, "ApiVersion Error: %s", sdrplay_api_GetErrorString(err));
+ SoapySDR_logf(SOAPY_SDR_ERROR, "sdrplay_api_ApiVersion() Error: %s", sdrplay_api_GetErrorString(err));
sdrplay_api_Close();
- throw std::runtime_error("ApiVersion() failed");
+ return;
}
if (ver != SDRPLAY_API_VERSION) {
- SoapySDR_logf(SOAPY_SDR_WARNING, "sdrplay_api version: '%.3f' does not equal build version: '%.3f'", ver, SDRPLAY_API_VERSION);
+ SoapySDR_logf(SOAPY_SDR_ERROR, "sdrplay_api version: '%.3f' does not equal build version: '%.3f'", ver, SDRPLAY_API_VERSION);
+ sdrplay_api_Close();
+ return;
}
+ valid = true;
}
SoapySDRPlay::sdrplay_api::~sdrplay_api()
{
- sdrplay_api_ErrT err;
+ if (!valid)
+ return;
+
// Close API
- err = sdrplay_api_Close();
+ sdrplay_api_ErrT err = sdrplay_api_Close();
if (err != sdrplay_api_Success) {
SoapySDR_logf(SOAPY_SDR_ERROR, "sdrplay_api_Close() failed: %s", sdrplay_api_GetErrorString(err));
}
Works for me. Josh needs to comment on what's the correct thing to do. |
|
@gvanem - first of all I apologize that my comment earlier about the problem to fix came out too strong. I am afraid that your proposed changes would create more problems than they are trying to fix. Imagine the scenario where the service For instance, take a look at this piece of code from the source file if the call to Also you may want to look at this topic in the C++ FAQ about the recommended way to handle constructors that fail: https://isocpp.org/wiki/faq/exceptions#ctors-can-throw Franco |
|
Okay so my fix is wrong. Then some other C++ expert could propose something better. |
The latest API from SDRPlay (released 5 days) has a change that causes a command like
SoapySDRUtil.exe --findto crashdeep inside
SoapySDR::Device::enumerate()'s destructor. The call-stack is huge.These are diffs for
sdrplay_api.h:The
sdrplay_api_DeviceTstructure is now 1 byte longer (unsigned char valid).Perhaps the
SoapySDRPlayXcode could check it's size at runtime to account for this breakage?The text was updated successfully, but these errors were encountered: