Next-Flip / Momentum-Firmware

🐬 Feature-rich, stable and customizable Flipper Firmware
https://momentum-fw.dev
GNU General Public License v3.0
4.9k stars 202 forks source link

NFC: OOM Crash #144

Open Untouched4373 opened 5 months ago

Untouched4373 commented 5 months ago

Describe the bug.

When the SubGHZ application is launched, and then the NFC application is used, the latter crashes and the Flipper Zero restarts.

Reproduction

  1. Launch the SubGHZ app.
  2. Exit the app and launch NFC app.

Target

Flipper Zero

Logs

No response

Anything else?

No response

Willy-JL commented 5 months ago

What version are you on?

Untouched4373 commented 5 months ago

Momentum-004

Untouched4373 commented 5 months ago

Okay, I found an extra condition for the problem to arise, you have to plug into the GPIO a CC1101 card .

octopus8321 commented 5 months ago

I haven't had that problem, try deleting everything and reinstalling

Willy-JL commented 5 months ago

what message does the crash give? does it still happen on a fresh install? please also try on dev version

mokeWe commented 4 months ago

I'm encountering the same issue in the latest dev build. However, the conditions to reproduce the bug are still elusive to me. It is always an out-of-memory error and seems to trigger more often if you open the NFC app directly after the Sub-Ghz app, but I can't replicate it reliably.

I'll post debug logs when I get the chance, but they show little to nothing of importance, as far as I can tell.

Willy-JL commented 4 months ago

Can you please try on unload-main-menu branch? Can find precompiled on GitHub actions tab or compile yourself

anotherlab commented 4 months ago

I am also getting a crash with some Mifare Classic cards. Using the latest public version, mmnt-004.

The cards read fine with the latest stock firmware (0.103.1).

Downloaded the artifact from build 627 and installed it from qFlipper. That crashed at the end of the install with the message

Flipper crashed 
and was rebooted
NULL pointer reference

After that, it had the normal Momentum display. I tried to read one of the problem cards and it crashed with the "out of memory" error. This firmware reports as MNTN-DEV, with a build date of 13 JUL 2024

mokeWe commented 4 months ago

Can you please try on unload-main-menu branch? Can find precompiled on GitHub actions tab or compile yourself

After a while, the NFC app still causes an OOM error. Later today, I'll replicate it and upload the log.

anotherlab commented 4 months ago

Is there anything that I can provide to help with the OOM error? I get 100% of the time with the Mifare Classic cards that I have.

mokeWe commented 4 months ago

Is there anything that I can provide to help with the OOM error? I get 100% of the time with the Mifare Classic cards that I have.

I haven't had any issues reading Mifare classic cards in the past, but I don't have any to test with right now. If you open another issue and post debug logs from the crash, I'm sure someone can help, but I'm not sure this OOM error is the same issue you're encountering. I hope you get that issue fixed!

Willy-JL commented 4 months ago

NULL pointer from @anotherlab is likely a different issue, and likely fixed in dev.

However, there's the OOM problem. Sadly, we are running out of usable RAM, the NFC app is huge and just keeps getting bigger, and some custom features of momentum require some baseline RAM usage in background.

The biggest one is asset packs. Animations are unloaded when opening apps, so this is not a problem. But icons and fonts must remain loaded when opening apps, so they keep some RAM occupied. Changing to an asset pack with less (or no) icons and fonts will usually "fix" this issue. Not a proper fix however, but there isn't much we can do to use less RAM, they just need to stay loaded.

Another thing that is kept in RAM when opening apps is main menu. On OFW, the app list is on firmware flash, while the main menu is in RAM. On momentum, due to the customizable app list, this is also in RAM, loaded when booting the flipper. In #161 I made it unload (most) of main menu components, and whole app list, when opening apps, and load it when first opening the menu or after exiting. From my testing, it's a minimal amount of loading time for a normal app set (10-20 apps) but hard to say how it will go on slower sd cards. If you click "reset menu" in momentum settings, it will not load from sd card at all, just load default app list from firmware flash. Anyway, unloading main menu and app list amounts to about 7kb of freed ram, so not much.

I'm still actively looking for solutions, but at this time it is looking quite bleak. Disabling your asset pack is always viable as a temporary fix however.

Willy-JL commented 4 months ago

As for logs, they are basically useless in this case. There's no secret what is happening here, it's not a software bug, it's simply too much stuff for the available RAM. It may be over time possible to reduce the ram usage to mostly fix this issue, but it's not a clear fix that can implemented right away.

mokeWe commented 4 months ago

I see. Thank you for clearing all of that up, and I hope over time this gets resolved. Until then, simply using the default asset pack won't be an issue. Thanks again :]

anotherlab commented 4 months ago

I had a couple of assets packs on the SD card and I wasn't any of them. When I delete them and left the default "Momentum" asset pack and then rebooted, I was able to read one of the cards that generated the OOM memory error.

What does this firmware add to the NFC over the default firmware? Is that documented anywhere?

Willy-JL commented 4 months ago

@anotherlab the "Momentum" asset pack is different from "Default". "Default" means no asset pack, it uses the icons built into the firmware, meaning no extra ram usage. The "Momentum" pack is separate (although included with momentum releases, it's not enabled by default and counts as a separate asset pack) and therefore uses more ram. Also, the momentum asset pack has more icons than the average asset pack, so it would usually use a bit more ram too. Anyway, deleting the asset packs was not necessary, simply switching the active one to "Default" is sufficient.

For the most part, the nfc app on momentum has EMV support (bank cards and the like), and some small file default naming tweaks that don't take up much. The EMV support does make the app a bit larger than on official firmware but not by that much. Official firmware has been adding new support recently (as you can see in the dev branch's CHANGELOG.md file) for different protocols, and this also takes considerable space. Simply put, there isn't much we can remove without feature loss, and we won't go that route unless strictly necessary.

anotherlab commented 4 months ago

@Willy-JL On my F0, I would get the OOM error every time until I removed the other asset packs. Weird. But part of life for memory issues. It all works fine until suddenly it doesn't.

Right now from the Momentum Start app, then Interface, then Graphics, Momentum is listed for the Asset Pack. I'll set it back to Default. I care more about reading the NFC cards than the icons.

Willy-JL commented 4 months ago

with these 2 PRs merged, i see about 11-12kb extra free ram, please try on latest dev build. still, having an asset pack enabled will use more ram than not having one, but overall there are some savings. also i will keep an eye out for similar optimizations to the ones in these PRs

anotherlab commented 4 months ago

@Willy-JL I installed the latest dev build (https://github.com/Next-Flip/Momentum-Firmware/actions/runs/9984631998), selected the Momentum asset pack, and then restarted the Flipper.

I am able to read completely one of the Mifare Classic 1K cards that caused the OOM error before without any errors..

AztecCodes commented 3 months ago

Check out this Issue: https://github.com/DarkFlippers/unleashed-firmware/issues/778

Probably the same problem.