Random application crash on Apple Silicon M1 (Qt 6.2.3)

KejPi

@J-Hilk said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

@KejPi said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

there are some GUI classes running in QThreads

mhm, is that something you have done?

Because GUI elements/classes are not allowed to run in a different thread from the one where QCoreApplication lives in!

I have not written it correctly, I did not want to go into details. My application consists of 2 parts - backend doing DAB demodulation and decoding, this is written in C and then the HMI part (frontend) that is written in C++ (Qt) and this controlling the backend, the input sources, doing audio and data decoding and playback, etc. And in the HMI part I have developed several classes and some of them run is separate thread but not those that have GUI elements, only some decodes, input devices, etc.

KejPi

@J-Hilk said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

woa, we're talking c++ here right? casting memory to a stuct is undefined behaviour, among other things because it circumvents the constructor! You get away with it in C but not in C++, thats way dynamic_cast exists

I do not do this :-) Trying to do it C++ way where it is possible but I have to admit I am trying to optimize all real-time code as much as possible - like using std::atomic for instead of mutex, etc.

DerReisende

@J-Hilk said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

@DerReisende said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

where you should memcpy received data instead of e.g. just casting to a struct which may cause problems.

woa, we're talking c++ here right? casting memory to a stuct is undefined behaviour, among other things because it circumvents the constructor! You get away with it in C but not in C++, thats way dynamic_cast exists

I have plenty of old code from former c programmers who used reinterpret_cast et al to convince the compiler to do what they wanted in really creative ways (created before y2k).
But anyways it was just (a maybe bad) example that the linked thread did not only talk about atomic ops.
If the OP doesnt do it - good. But still a compile run with the undefined behaviour sanitizer may find some problems that may fix the intermittent crashes of the app.

KejPi

I have compiled my app with undefined behaviour sanitizer. So far I get a lot of messages like this (not sure how to get rid of them) - they are generated for every Q_OBJECT class I have created:

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/kejpi/Devel/dab/build-gui-Qt_6_2_3-Debug/AbracaDABra_autogen/EWIEGA46WW/moc_audiodecoder.cpp:172:28 in 
/Users/kejpi/Devel/dab/build-gui-Qt_6_2_3-Debug/AbracaDABra_autogen/EWIEGA46WW/moc_slideshowapp.cpp:145:28: runtime error: member access within address 0x6000025a1d00 which does not point to an object of type 'QObjectData'
0x6000025a1d00: note: object is of type 'QObjectPrivate'
 00 00 00 00  90 0d 18 08 01 00 00 00  c0 ef eb 01 00 60 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'QObjectPrivate'

And then I was able to find 2 minor issues in my code (shifting of negative number and undefined enum value that was not used anyway). I have also found 1 misalignment that may cause problems. I have fixed all 3 issues and still the application is crashing with message like this:

UndefinedBehaviorSanitizer:DEADLYSIGNAL
==3606==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000000000 bp 0x0001be9aaf8c sp 0x00016da86520 T42532)
==3606==Hint: pc points to the zero page.
==3606==The signal is caused by a UNKNOWN memory access.
==3606==Hint: address points to the zero page.
    #0 0x0  (<unknown module>)

==3606==Register values:
 x[0] = 0x00000001beba354f   x[1] = 0x0000000000000000   x[2] = 0x00000001bea02d4c   x[3] = 0x00000001bea02da4  
 x[4] = 0x00000001548a9118   x[5] = 0x00000001548a9128   x[6] = 0x00000001548a9210   x[7] = 0x0000600000b8e800  
 x[8] = 0x0000000000000003   x[9] = 0x0000000000000003  x[10] = 0x000000000000001d  x[11] = 0x0000000000000000  
x[12] = 0x0000000000000005  x[13] = 0x0000000155087330  x[14] = 0x00000001beba7028  x[15] = 0x000000020e105458  
x[16] = 0x0000000000000000  x[17] = 0x000000020fdb2b90  x[18] = 0x0000000000000000  x[19] = 0x000000020fdaacb0  
x[20] = 0x00000001548a9128  x[21] = 0x00000001548a9210  x[22] = 0x0000600001efe240  x[23] = 0x0000600000b8eae0  
x[24] = 0x000060000378d600  x[25] = 0x0000600000b8e840  x[26] = 0x0000600002fd1200  x[27] = 0x0000600002fd0fc0  
x[28] = 0x0000600003297020     fp = 0x000000016da868b0     lr = 0x00000001be9aaf8c     sp = 0x000000016da86520  
UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: SEGV (<unknown module>) 
==3606==ABORTING

My reading is that it accessing of NULL pointer somewhere.

DerReisende

Maybe -fsanitize=thread for thread sanitizer is worth a try as well as it is supposed to find data races (Docs).
Otherwise I am running out of ideas.

DerReisende

@KejPi Hmm, I found this document which may be useful from Apple.

As far as I understand ARM stores the return address in the link register. Therefore if I understood correctly the lr register of your stacktrace should point to the code where the fault was triggered. Maybe this applies to Apple Silicon as well…

KejPi

I have tried to run it with thread sanitizer and there is quite a log of data races reported, mostly related to the way I share data from backend (C library) and HMI/Application (Qt). It makes me think that maybe I have some systematic issue :-( This is what I have:

Backend is C library that runs in Posix thread (T1)
Data from backend is passed using callback function
There is dedicated class called RadioControl that is running in QThread (T2). This class does all the communication with backend and communicates with other classes by signals
Callback functions are declared as friend to RadioControl (static function should work as well IMO)
I need to take the data in callback function (that is running in T1) and pass them to RadioControl thread T2. I am doing it using signal and Qt::QueuedConnection inside RadioControl class.

Implementation extract:

radiocontrol.h

class RadioControl : public QObject
{
    Q_OBJECT
public:
    bool init();
    // ...
signals:
    void dabEvent(RadioControlEvent * pEvent);
    // ...
private:
    void eventFromDab(RadioControlEvent * pEvent);
    void emit_dabEvent(RadioControlEvent * pEvent) { emit dabEvent(pEvent); }
    friend void dabNotificationCb(dabProcNotificationCBData_t * p, void * ctx);
};

radiocontrol.cpp

bool RadioControl::init()
{   
    connect(this, &RadioControl::dabEvent, this, &RadioControl::eventFromDab, Qt::QueuedConnection);

    // passing this in last argument as context (ctx)
    dabProcRegisterNotificationCb(dabProcHandle, dabNotificationCb, (void *) this);  
    //...
}

void RadioControl::eventFromDab(RadioControlEvent * pEvent)
{
    switch (pEvent->type)  // <=== sanitizer complains that this is the data race
    {
    case RadioControlEventType::SYNC_STATUS:
    {
    }
    break;
   //...
}

void dabNotificationCb(dabProcNotificationCBData_t * p, void * ctx)
{
    RadioControl * radioCtrl = static_cast<RadioControl *>(ctx);
    switch (p->nid)
    {
    case DABPROC_NID_SYNC_STATUS:
    {
        RadioControlEvent * pEvent = new RadioControlEvent;
        const dabProc_NID_SYNC_STATUS_t * pInfo = static_cast<const dabProc_NID_SYNC_STATUS_t *>(p->pData); 

        pEvent->type = RadioControlEventType::SYNC_STATUS;    // <=== sanitizer complains that this is the data race

        pEvent->status = p->status;        
        pEvent->pData = static_cast<intptr_t>(pInfo->syncLevel);
        radioCtrl->emit_dabEvent(pEvent);
    }
        break;
    // ...
}

Do you see any principal issue with this approach?

J.Hilk

@KejPi

void RadioControl::eventFromDab(RadioControlEvent * pEvent)
{
switch (pEvent->type) // <=== sanitizer complains that this is the data race

this one is, you're passing a pointer to an object around, that lives on an other thread. The QueuedConnecion will only copy the pointer not the underlying data.

KejPi

@J-Hilk said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

@KejPi

void RadioControl::eventFromDab(RadioControlEvent * pEvent)
{
switch (pEvent->type) // <=== sanitizer complains that this is the data race

this one is, you're passing a pointer to an object around, that lives on an other thread. The QueuedConnecion will only copy the pointer not the underlying data.

Yes, that is actually my point. I do not want to do copy of the data (in this case it is small, but it can be significantly bigger), I want to pass a pointer to the other thread that reads the data and finally deletes them. Data in callback are not touched after radioCtrl->emit_dabEvent(pEvent).

If this is not the correct way, how to pass the data without additional copying correctly? Would it be better to use QSharedPointer<RadioControlEvent> instead?

RadioControlEvent is this structure:

struct RadioControlEvent
{
    RadioControlEventType type;          // <-- enum class RadioControlEventType
    dabProcNotificationStatus_t status;  // <-- enum
    intptr_t pData;
};

J.Hilk

@KejPi oh my, you're handling Nitroglycerin, when you could work with the much saver Dynamite !

if you catch my allegory :D

use a mutex!
https://en.cppreference.com/w/cpp/thread/mutex
https://doc.qt.io/qt-5/qmutex.html

KejPi

@J-Hilk said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

@KejPi oh my, you're handling Nitroglycerin, when you could work with the much saver Dynamite !

if you catch my allegory :D

use a mutex!
https://en.cppreference.com/w/cpp/thread/mutex
https://doc.qt.io/qt-5/qmutex.html

I am used to develop bare metal DSP code in very low level C - I like working with explosives :-D
I understand that mutex is the right way to protect the data but it is not for free and it may potentially block for some time the real time processing in backend library that I want to avoid. My intention is to have callback as fast as possible just to pass data and continue with processing of the data that is not waiting. It will not be easy to hit this goal with mutex :-(

J.Hilk

@KejPi

I am used to develop bare metal DSP code in very low level C - I like working with explosives :-D

:D

I understand that mutex is the right way to protect the data but it is not for free and it may potentially block for some time the real time processing in backend library that I want to avoid. My intention is to have callback as fast as possible just to pass data and continue with processing of the data that is not waiting. It will not be easy to hit this goal with mutex

If you're working across threads with non atomic types, you'll either have to copy stuff, or use a mutex, there is no other (save) way.

My advice, do an actual benchmark of a mutex, see what difference in time it actually does.

KejPi

@J-Hilk I will try to insert mutex in the code to check if it solves the issue with application crash but I still to not think this is a problem because by principle only one thread is accessing the data that is created and set in callback and then sent by signal.

DerReisende

@KejPi said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

Yes, that is actually my point. I do not want to do copy of the data (in this case it is small, but it can be significantly bigger), I want to pass a pointer to the other thread that reads the data and finally deletes them. Data in callback are not touched after radioCtrl->emit_dabEvent(pEvent).

If this is not the correct way, how to pass the data without additional copying correctly? Would it be better to use QSharedPointer<RadioControlEvent> instead?

Isn‘t this a „perfect“ use-case for a queue? T1 writes the data to the queue and signals T2 that data is available. T2 then retrieves the data from the queue, processes and deletes it.
Boost.Lockfree queue or spsc_queue (single producer, single consumer) come into my mind here, I don‘t know if Qt offers similar thread-safe classes (std::queue does not seem to be thread-safe).

KejPi

@DerReisende Yes, it is exactly one producer-on consumer queue. Actually my idea was to "emulate" it by Qt::QueuedConnection. But in theory I can implement simple queue by myself since it is quite simple case. Nonetheless the problem mentioned by @J-Hilk that data live in T1 that creates them and used by T2 that uses and deletes them remains. But queue is a good idea how to solve it :-)

DerReisende

@KejPi said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

@DerReisende Yes, it is exactly one producer-on consumer queue. Actually my idea was to "emulate" it by Qt::QueuedConnection. But in theory I can implement simple queue by myself since it is quite simple case.

Why reinvent the wheel when there are proven/well-tested solutions available? :D

Nonetheless the problem mentioned by @J-Hilk that data live in T1 that creates them and used by T2 that uses and deletes them remains. But queue is a good idea how to solve it :-)

What about using move semantics (std::move)? You should be able to move your RadioControlEvent in there and later move it out of the queue. But it would then require a move constructor.

artwaw

@KejPi Qt has QQueue, which is reentrant and thread-safe to some extent, https://doc.qt.io/qt-6/qqueue.html - but I am not sure if this will suit your needs. I used it, upon occasion, for the file processing backend to communicate/report to the UI frontend however my constraints were not as tight as yours I am afraid.

KejPi

@DerReisende said in Random application crash on Apple Silicon M1 (Qt 6.2.3):

What about using move semantics (std::move)? You should be able to move your RadioControlEvent in there and later move it out of the queue. But it would then require a move constructor.

Yes, but this implies Qt6, so far I am able to build my app for both Qt5 and Qt6 but maybe it is time to move forward.

@artwaw You are probably right, I need really light and fast solution, it is of course no problem to run the application on M1 but the target is to be able to run it on RPi2.

artwaw

@KejPi QList and QQueue are lightweight, at least claim to be in the documentation. I never had memory problems on raspi, then again I never did anything that would strain the resources on raspi2.

Having said that I trust there more experienced persons in this thread that would be able to judge if QList/Queue is the way to go in terms of usability or not.

DerReisende

@artwaw QList is reentrant but not thread-safe. At least it is not mentioned in the docs and there are several questions over at stackoverflow.