altosui: Close serial, join reader thread, free altos_file

Separating out the close and free actions ensures that the reader thread will not
access freed memory or dereference a null pointer while shutting down the
connection to the serial device. Otherwise, a race condition exists between the
serial close and the thread join.

Signed-off-by: Keith Packard <keithp@keithp.com>

diff --git a/ao-tools/altosui/AltosSerial.java b/ao-tools/altosui/AltosSerial.java
index 96e8b61f599d2329678dbe547b3edc3eb3389f11..efa63f681819e2ed9f3d22ebfac66d79e47f7750 100644 (file)
--- a/ao-tools/altosui/AltosSerial.java
+++ b/ao-tools/altosui/AltosSerial.java
@@ -51,8 +51,6 @@ public class AltosSerial implements Runnable {
 
                try {
                        for (;;) {
-                               if (altos == null)
-                                       break;
                                c = libaltos.altos_getchar(altos, 0);
                                if (Thread.interrupted())
                                        break;
@@ -106,10 +104,8 @@ public class AltosSerial implements Runnable {
        }
 
        public void close() {
-               if (altos != null) {
+               if (altos != null)
                        libaltos.altos_close(altos);
-                       altos = null;
-               }
                if (input_thread != null) {
                        try {
                                input_thread.interrupt();
@@ -118,6 +114,10 @@ public class AltosSerial implements Runnable {
                        }
                        input_thread = null;
                }
+               if (altos != null) {
+                       libaltos.altos_free(altos);
+                       altos = null;
+               }
        }
 
        public void putc(char c) {

diff --git a/ao-tools/libaltos/libaltos.c b/ao-tools/libaltos/libaltos.c
index df0d5b2eba6b21f99ea35e9196bed7771565cd6a..00fb2125443bc452b9807f8f106d81b726dd30b9 100644 (file)
--- a/ao-tools/libaltos/libaltos.c
+++ b/ao-tools/libaltos/libaltos.c
@@ -567,6 +567,14 @@ void
 altos_close(struct altos_file *file)
 {
        close(file->fd);
+       file->fd = -1;
+}
+
+void
+altos_free(struct altos_file *file)
+{
+       if (file->fd != -1)
+               close(file->fd);
        free(file);
 }
 
@@ -592,6 +600,8 @@ altos_flush(struct altos_file *file)
        while (file->out_used) {
                int     ret;
 
+               if (file->fd < 0)
+                       return -EBADF;
                ret = write (file->fd, file->out_data, file->out_used);
                if (ret < 0)
                        return -errno;
@@ -610,6 +620,8 @@ altos_getchar(struct altos_file *file, int timeout)
                int     ret;
 
                altos_flush(file);
+               if (file->fd < 0)
+                       return -EBADF;
                ret = read(file->fd, file->in_data, USB_BUF_SIZE);
                if (ret < 0)
                        return -errno;

diff --git a/ao-tools/libaltos/libaltos.h b/ao-tools/libaltos/libaltos.h
index 782f244eb6dd68b7682744f2ee43c2e70b3bce45..53026e0a755fa4d16609d27ec15c68602d15f47b 100644 (file)
--- a/ao-tools/libaltos/libaltos.h
+++ b/ao-tools/libaltos/libaltos.h
@@ -42,6 +42,8 @@ altos_open(struct altos_device *device);
 
 void altos_close(struct altos_file *file);
 
+void altos_free(struct altos_file *file);
+
 int
 altos_putchar(struct altos_file *file, char c);