mcu: Be careful to free memory allocated in C code

Free steppersync, stepcompress, and commandqueue objects.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>

klippy/chelper.py

@@ -15,6 +15,7 @@ defs_stepcompress = """
     struct stepcompress *stepcompress_alloc(uint32_t max_error
         , uint32_t queue_step_msgid, uint32_t set_next_step_dir_msgid
         , uint32_t invert_sdir, uint32_t oid);
+    void stepcompress_free(struct stepcompress *sc);
     void stepcompress_push(struct stepcompress *sc, double step_clock
         , int32_t sdir);
     int32_t stepcompress_push_factor(struct stepcompress *sc
@@ -38,6 +39,7 @@ defs_stepcompress = """
 
     struct steppersync *steppersync_alloc(struct serialqueue *sq
         , struct stepcompress **sc_list, int sc_num, int move_num);
+    void steppersync_free(struct steppersync *ss);
     void steppersync_flush(struct steppersync *ss, uint64_t move_clock);
 """
 
@@ -53,6 +55,7 @@ defs_serialqueue = """
     void serialqueue_exit(struct serialqueue *sq);
     void serialqueue_free(struct serialqueue *sq);
     struct command_queue *serialqueue_alloc_commandqueue(void);
+    void serialqueue_free_commandqueue(struct command_queue *cq);
     void serialqueue_send(struct serialqueue *sq, struct command_queue *cq
         , uint8_t *msg, int len, uint64_t min_clock, uint64_t req_clock);
     void serialqueue_encode_and_send(struct serialqueue *sq

klippy/mcu.py

@@ -42,9 +42,10 @@ class MCU_stepper:
         self._reset_cmd = mcu.lookup_command(
             "reset_step_clock oid=%c clock=%u")
         ffi_main, self.ffi_lib = chelper.get_ffi()
-        self._stepqueue = self.ffi_lib.stepcompress_alloc(
+        self._stepqueue = ffi_main.gc(self.ffi_lib.stepcompress_alloc(
             max_error, self._step_cmd.msgid
-            , self._dir_cmd.msgid, self._invert_dir, self._oid)
+            , self._dir_cmd.msgid, self._invert_dir, self._oid),
+                                      self.ffi_lib.stepcompress_free)
         self.print_to_mcu_time = mcu.print_to_mcu_time
     def get_oid(self):
         return self._oid
@@ -356,6 +357,9 @@ class MCU:
             self.get_print_buffer_time = dummy_get_print_buffer_time
     def disconnect(self):
         self.serial.disconnect()
+        if self._steppersync is not None:
+            self.ffi_lib.steppersync_free(self._steppersync)
+            self._steppersync = None
     def stats(self, eventtime):
         stats = self.serial.stats(eventtime)
         stats += " mcu_task_avg=%.06f mcu_task_stddev=%.06f" % (
@@ -484,3 +488,5 @@ class MCU:
         mcu_time = print_time + self._print_start_time
         clock = int(mcu_time * self._mcu_freq)
         self.ffi_lib.steppersync_flush(self._steppersync, clock)
+    def __del__(self):
+        self.disconnect()

klippy/serialhdl.py

@@ -112,8 +112,6 @@ class SerialReader:
     def disconnect(self):
         if self.serialqueue is None:
             return
-        self.send_flush()
-        time.sleep(0.010)
         self.ffi_lib.serialqueue_exit(self.serialqueue)
         if self.background_thread is not None:
             self.background_thread.join()
@@ -170,7 +168,8 @@ class SerialReader:
     def send_flush(self):
         self.ffi_lib.serialqueue_flush_ready(self.serialqueue)
     def alloc_command_queue(self):
-        return self.ffi_lib.serialqueue_alloc_commandqueue()
+        return self.ffi_main.gc(self.ffi_lib.serialqueue_alloc_commandqueue(),
+                                self.ffi_lib.serialqueue_free_commandqueue)
     # Dumping debug lists
     def dump_debug(self):
         sdata = self.ffi_main.new('struct pull_queue_message[1024]')
@@ -222,6 +221,8 @@ class SerialReader:
         logging.info("%s: %s" % (params['#name'], params['#msg']))
     def handle_default(self, params):
         logging.warn("got %s" % (params,))
+    def __del__(self):
+        self.disconnect()
 
 # Class to retry sending of a query command until a given response is received
 class SerialRetryCommand:

klippy/serialqueue.c

@@ -130,6 +130,18 @@ pollreactor_setup(struct pollreactor *pr, int num_fds, int num_timers
         pr->timers[i].waketime = PR_NEVER;
 }
 
+// Free resources associated with a 'struct pollreactor' object
+static void
+pollreactor_free(struct pollreactor *pr)
+{
+    free(pr->fds);
+    pr->fds = NULL;
+    free(pr->fd_callbacks);
+    pr->fd_callbacks = NULL;
+    free(pr->timers);
+    pr->timers = NULL;
+}
+
 // Add a callback for when a file descriptor (fd) becomes readable
 static void
 pollreactor_add_fd(struct pollreactor *pr, int pos, int fd, void *callback)
@@ -358,6 +370,18 @@ message_free(struct queue_message *qm)
     free(qm);
 }
 
+// Free all the messages on a queue
+void
+message_queue_free(struct list_head *root)
+{
+    while (!list_empty(root)) {
+        struct queue_message *qm = list_first_entry(
+            root, struct queue_message, node);
+        list_del(&qm->node);
+        message_free(qm);
+    }
+}
+
 
 /****************************************************************
  * Serialqueue interface
@@ -438,18 +462,6 @@ debug_queue_add(struct list_head *root, struct queue_message *qm)
     message_free(old);
 }
 
-// Free all the messages on a queue
-static void
-queue_free(struct list_head *root)
-{
-    while (!list_empty(root)) {
-        struct queue_message *qm = list_first_entry(
-            root, struct queue_message, node);
-        list_del(&qm->node);
-        message_free(qm);
-    }
-}
-
 // Wake up the receiver thread if it is waiting
 static void
 check_wake_receive(struct serialqueue *sq)
@@ -863,21 +875,24 @@ serialqueue_exit(struct serialqueue *sq)
 void
 serialqueue_free(struct serialqueue *sq)
 {
+    if (!sq)
+        return;
     if (!pollreactor_is_exit(&sq->pr))
         serialqueue_exit(sq);
     pthread_mutex_lock(&sq->lock);
-    queue_free(&sq->sent_queue);
+    message_queue_free(&sq->sent_queue);
-    queue_free(&sq->receive_queue);
+    message_queue_free(&sq->receive_queue);
-    queue_free(&sq->old_sent);
+    message_queue_free(&sq->old_sent);
-    queue_free(&sq->old_receive);
+    message_queue_free(&sq->old_receive);
     while (!list_empty(&sq->pending_queues)) {
         struct command_queue *cq = list_first_entry(
             &sq->pending_queues, struct command_queue, node);
         list_del(&cq->node);
-        queue_free(&cq->ready_queue);
+        message_queue_free(&cq->ready_queue);
-        queue_free(&cq->stalled_queue);
+        message_queue_free(&cq->stalled_queue);
     }
     pthread_mutex_unlock(&sq->lock);
+    pollreactor_free(&sq->pr);
     free(sq);
 }
 
@@ -892,6 +907,19 @@ serialqueue_alloc_commandqueue(void)
     return cq;
 }
 
+// Free a 'struct command_queue'
+void
+serialqueue_free_commandqueue(struct command_queue *cq)
+{
+    if (!cq)
+        return;
+    if (!list_empty(&cq->ready_queue) || !list_empty(&cq->stalled_queue)) {
+        fprintf(stderr, "Memory leak! Can't free non-empty commandqueue\n");
+        return;
+    }
+    free(cq);
+}
+
 // Add a batch of messages to the given command_queue
 void
 serialqueue_send_batch(struct serialqueue *sq, struct command_queue *cq

klippy/serialqueue.h

@@ -35,6 +35,7 @@ struct queue_message {
 };
 
 struct queue_message *message_alloc_and_encode(uint32_t *data, int len);
+void message_queue_free(struct list_head *root);
 
 struct pull_queue_message {
     uint8_t msg[MESSAGE_MAX];
@@ -47,6 +48,7 @@ struct serialqueue *serialqueue_alloc(int serial_fd, int write_only);
 void serialqueue_exit(struct serialqueue *sq);
 void serialqueue_free(struct serialqueue *sq);
 struct command_queue *serialqueue_alloc_commandqueue(void);
+void serialqueue_free_commandqueue(struct command_queue *cq);
 void serialqueue_send_batch(struct serialqueue *sq, struct command_queue *cq
                             , struct list_head *msgs);
 void serialqueue_send(struct serialqueue *sq, struct command_queue *cq

klippy/stepcompress.c

@@ -290,6 +290,17 @@ stepcompress_alloc(uint32_t max_error, uint32_t queue_step_msgid
     return sc;
 }
 
+// Free memory associated with a 'stepcompress' object
+void
+stepcompress_free(struct stepcompress *sc)
+{
+    if (!sc)
+        return;
+    free(sc->queue);
+    message_queue_free(&sc->msg_queue);
+    free(sc);
+}
+
 // Convert previously scheduled steps into commands for the mcu
 static void
 stepcompress_flush(struct stepcompress *sc, uint64_t move_clock)
@@ -573,6 +584,18 @@ steppersync_alloc(struct serialqueue *sq, struct stepcompress **sc_list
     return ss;
 }
 
+// Free memory associated with a 'steppersync' object
+void
+steppersync_free(struct steppersync *ss)
+{
+    if (!ss)
+        return;
+    free(ss->sc_list);
+    free(ss->move_clocks);
+    serialqueue_free_commandqueue(ss->cq);
+    free(ss);
+}
+
 // Implement a binary heap algorithm to track when the next available
 // 'struct move' in the mcu will be available
 static void