flipperdevices / flipperzero-firmware

Flipper Zero firmware source code
https://flipperzero.one
GNU General Public License v3.0
12.66k stars 2.7k forks source link

Flipper crash with custom NotificationSequence #2313

Closed dcoles closed 1 year ago

dcoles commented 1 year ago

Describe the bug.

When a user-defined NotificationSequence is run at the end of an external app using the non-blocking notification_sequence API, the Flipper Zero will crash ~50% due to a Bus fault or NULL pointer dereference.

Reproduction

External application used for reproduction (firmware: 0.74.2)

#include <furi.h>
#include <notification/notification.h>
#include <notification/notification_messages.h>

const NotificationSequence chime = {
    &message_note_c4,
    &message_delay_100,
    &message_note_e4,
    &message_delay_100,
    &message_note_g4,
    &message_delay_100,
    &message_note_c5,
    &message_delay_100,
    &message_sound_off,
    NULL
};

int hello_world(char* arg) {
    (void) arg;

    NotificationApp* notifications = furi_record_open(RECORD_NOTIFICATION);
    notification_message(notifications, &chime);
    furi_record_close(RECORD_NOTIFICATION);

    return 0;
}

Target

f7

Logs

2783288 [I][fap_loader_app] FAP Loader is loading /ext/apps/Examples/hello_world.fap
2783338 [I][fap_loader_app] FAP Loader is mapping
2783388 [I][elf] Total size of loaded sections: 104
2783390 [I][fap_loader_app] Loaded in 102ms
2783393 [I][fap_loader_app] FAP Loader is starting app
2783407 [I][fap_loader_app] FAP app returned: 0
2783413 [D][BrowserWorker] Start
2783449 [D][BrowserWorker] Enter folder: /ext/apps/Examples items: 9 idx: 1
2783451 [D][BrowserWorker] Load offset: 0 cnt: 50
<Serial client disconnected>
skotopes commented 1 year ago

That is expected behavior: notification_message is asynchronous function and if app exited and sequence is freed it will crash.

I'll update documentation and check what we can do to make it easier to use.

dcoles commented 1 year ago

I was wondering if that was the case (particularly since notification_message_block exists).

Unfortunately, this makes notification_message tricky to use safely in external apps. A user would need to ensure their application waits if a notification message was still in progress. Not that it can't be done, but it's an easy mistake to make.

Out of curiosity, do notification sequences queue up? If so, could notification_sequence_block be used as a way to ensure all prior notification sequences are finished?

skotopes commented 1 year ago

Main use case for notification_sequence_block is to wait till execution completed in a places where you may need it. And yes it can be used for that.

DrZlo13 commented 1 year ago

Fixed in #2335. Now you do not need a blocking notification when you exit the application.