From: Keith Packard <keithp@keithp.com>
Date: Thu, 29 Jul 2010 17:45:02 +0000 (-0700)
Subject: altosui: Close serial, join reader thread, free altos_file
X-Git-Tag: debian/0.6+290+g7877496~10
X-Git-Url: https://git.gag.com/?p=fw%2Faltos;a=commitdiff_plain;h=53c279b9e96da8b69837ae84038a78ca5707f2a5;hp=b8bc9994d8bfde6116c8a509e70ddf45fc4decce

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 96e8b61f..efa63f68 100644
--- 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 df0d5b2e..00fb2125 100644
--- 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 782f244e..53026e0a 100644
--- 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);