csete / gpredict

Gpredict satellite tracking application
http://gpredict.oz9aec.net/
GNU General Public License v2.0
829 stars 245 forks source link

Closing the rotor control window while a rotor is engaged no longer c… #315

Closed KaComet closed 6 months ago

KaComet commented 1 year ago

…rashes Gpredict. The issue is with how the rot_ctrl_timeout_cb was assigned to a timeout and was documented in #279. The timeout runs the callback function everytime the delay timer runs out. This normally allows for the rotor/display to be updated regularly, with the update interval defined by the user in the "cycle" box.

The problem originates from a duplicate timer that is created in gtk_rot_ctrl_new. When gtk_rot_ctrl_new sets up the widgets. it links the delay_changed_cb to the "cycle" box. Somewhat unintuitively, this action results in delay_changed_cb being called immediately. Every time delay_changed_cb is run, it will delete any existing timeout and create a new one. Later, gtk_rot_ctrl_new creates an additional timeout and forgets the ID of the timeout made by the initial running of the delay_changed_cb. This forgotten timeout will run rot_ctrl_timeout_cb forever. The reason this issue was only observed while the rotor is engaged was because rot_ctrl_timeout_cb can only access freed resources when ctrl->engaged is true.

The solution is to remove the line that creates a rot_ctrl_timeout_cb from gtk_rot_ctrl_new and rely on the initial running of delay_changed_cb to create the timeout.

TL;DR PR corrects memory leak in the rotor control window that sometimes results in a crash.

csete commented 6 months ago

Thanks for the fix @KaComet.