Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix crash during reset nodedb #4017

Merged
merged 1 commit into from
Jun 2, 2024
Merged

fix crash during reset nodedb #4017

merged 1 commit into from
Jun 2, 2024

Conversation

mverch67
Copy link
Collaborator

@mverch67 mverch67 commented Jun 2, 2024

This PR fixes a crash during reset NodeDB (dereferencing null pointer).

The order of statements in resetNodes() is wrong. First, numMeshNodes is set to 1, but after that (in clearLocalPosition()) the nodeId is searched in the meshDB, which cannot be found anymore, because meshDB is set to one element already.

Note: normally this may not happen, as the own node is at position 0 (the only one element which is found). But when running MQTT with hundreds of nodes the own node seems not at position 0 anymore -> crash.

Another question is: who wrote code that reorders the own node away from position 0? Should not be necessary at all, right?

@mverch67
Copy link
Collaborator Author

mverch67 commented Jun 2, 2024

After some thinking I have an idea what the root cause is that the own node is not at position 0.
After receiving the admin message and purging meshDB there are upto 7s until the node reboots. If new nodes are advertised (as in case of MQTT happens every second) they are inserted into the db. So after the reboot these new nodes are already in the nodeDB. Also, if for some reason the own nodeId is regenerated (e.g. using parameter -h on native) the own node will not occupy position 0 anymore.

Another problem I saw is now the own nodeId will never be at position 0 again, because the line

std::fill(devicestate.node_db_lite.begin() + 1, devicestate.node_db_lite.end(), meshtastic_NodeInfoLite());

is not removing the actual node at position 0 !! So at next reboot our own node will be at position 1 in the meshDB.

Anyway, the fix in this PR prevents the crash as a result of the moving of the own node away from position 0.

@thebentern thebentern merged commit 2723ae6 into master Jun 2, 2024
82 checks passed
@thebentern thebentern deleted the fix-crash-reset-nodedb branch June 2, 2024 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants