From cea38abf1f76b5b795b9341406ee814303f5752b Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Thu, 19 Aug 2010 19:25:58 -0700
Subject: [PATCH] avoid double close() and EBADF

It can be dangerous to hit (and ignore) EBADF errors in
multi-threaded applications.  Users of POSIX_MQ#to_io have two
Ruby objects pointing to the same file descriptor, making
things tricky when it comes time to reap resources.

We'll always prefer to close the Ruby IO object if it exists
(because we have less control over its GC behavior) and
ignore the raw descriptor.
---
 ext/posix_mq/posix_mq.c | 38 +++++++++++++++++++++++++-------------
 test/test_posix_mq.rb   |  1 +
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/ext/posix_mq/posix_mq.c b/ext/posix_mq/posix_mq.c
index 04c1ea0..73d139b 100644
--- a/ext/posix_mq/posix_mq.c
+++ b/ext/posix_mq/posix_mq.c
@@ -29,11 +29,8 @@
 #else
 #  define MQ_IO_MARK(mq) ((void)(0))
 #  define MQ_IO_SET(mq,val) ((void)(0))
-#endif
-
-#ifdef MQD_TO_FD
-#  define MQ_IO_MARK(mq) rb_gc_mark((mq)->io)
-#  define MQ_IO_SET(mq,val) do { (mq)->io = (val); } while (0)
+#  define MQ_IO_CLOSE(mq) ((void)(0))
+#  define MQ_IO_NILP(mq) ((void)(1))
 #endif
 
 struct posix_mq {
@@ -46,6 +43,23 @@ struct posix_mq {
 #endif
 };
 
+#ifdef MQD_TO_FD
+#  define MQ_IO_MARK(mq) rb_gc_mark((mq)->io)
+#  define MQ_IO_SET(mq,val) do { (mq)->io = (val); } while (0)
+#  define MQ_IO_NIL_P(mq) NIL_P((mq)->io)
+static int MQ_IO_CLOSE(struct posix_mq *mq)
+{
+	if (NIL_P(mq->io))
+		return 0;
+
+	/* not safe during GC */
+	rb_io_close(mq->io);
+	mq->io = Qnil;
+
+	return 1;
+}
+#endif
+
 static VALUE cPOSIX_MQ, cAttr;
 static ID id_new, id_kill, id_fileno;
 static ID sym_r, sym_w, sym_rw;
@@ -205,12 +219,10 @@ static void _free(void *ptr)
 {
 	struct posix_mq *mq = ptr;
 
-	if (mq->des != MQD_INVALID) {
+	if (mq->des != MQD_INVALID && MQ_IO_NIL_P(mq)) {
 		/* we ignore errors when gc-ing */
-		int saved_errno = errno;
-
 		mq_close(mq->des);
-		errno = saved_errno;
+		errno = 0;
 	}
 	xfree(ptr);
 }
@@ -667,11 +679,11 @@ static VALUE _close(VALUE self)
 {
 	struct posix_mq *mq = get(self, 1);
 
-	if (mq_close(mq->des) < 0)
-		rb_sys_fail("mq_close");
-
+	if (! MQ_IO_CLOSE(mq)) {
+		if (mq_close(mq->des) == -1)
+			rb_sys_fail("mq_close");
+	}
 	mq->des = MQD_INVALID;
-	MQ_IO_SET(mq, Qnil);
 
 	return Qnil;
 }
diff --git a/test/test_posix_mq.rb b/test/test_posix_mq.rb
index 37ca664..78a005b 100644
--- a/test/test_posix_mq.rb
+++ b/test/test_posix_mq.rb
@@ -30,6 +30,7 @@ class Test_POSIX_MQ < Test::Unit::TestCase
   def test_gc
     assert_nothing_raised do
       2025.times { POSIX_MQ.new(@path, :rw) }
+      2025.times { a = POSIX_MQ.new(@path, :rw); a.to_io }
     end
   end
 
-- 
2.11.4.GIT