Re: [Openais] Patch for WhiteTank to prevent shared memory leak

[Jérôme Flesch]

This patch is definitively about fixing a bug due to a BSD-ism.

My test case:
On a FreeBSD system:
- a client connects to Aisexec and creates the SHM
- the client does its job
- the client disconnects, the SHM is not destroyed by Aisexec

After some debugging, I've found out that, at the disconnection, the connection ref counter was incremented at ipc.c:~622 (right after the "if (revent & POLLIN)") but didn't appear to be decremented after that. When you look at the comment at ipc.c:~640, it states that, on BSD systems, EOF are detected not by poll() but by read(). So on a BSD system, when a client disconnects, read() will return 0. It means that it won't go through the block "if (res == 1) { ... }", and so won't call openais_conn_refcount_dec(). However, on a Linux system, we are sure that read() will always return 1 since poll() told us there is something to read and we only ask for 1 byte, and so we are sure that openais_conn_refcount_dec() will always be called. This patch just ensures that whatever read() returns, we call openais_conn_refcount_dec() to compensate for the initial call to openais_conn_refcount_inc(). It also removes the "res = 0;" in the default case of the switch (ie when the message is not recognized), because on a Linux system it doesn't have any effect, but on a BSD system it will disconnect the client. It didn't seem right to make Aisexec have a different behavior on BSD than on Linux.

I don't think this is a proper fix as it would break the proper
refcounting on Linux systems.  I have tested conn_info_destroy pretty
extensively on Linux and it works well.  Could you explain your test
case?  This may be a bsd-ism that we need to work around.

Regards
-steve
On Wed, 2009-04-15 at 16:02 +0200, Jérôme Flesch wrote:

Hi,




plain text document attachment (whitetank-fix-ipc-shm-leak.patch)
diff -ru openais-whitetank-9-old/exec/ipc.c openais-whitetank-9/exec/ipc.c
--- exec/ipc.c  2009-04-15 15:08:23.000000000 +0200
+++ exec/ipc.c  2009-04-15 15:09:54.000000000 +0200
@@ -630,7 +630,6 @@
                                res = 0;
                                break;
                        }
-                       openais_conn_refcount_dec (conn_info);
                }
 #if defined(OPENAIS_SOLARIS) || defined(OPENAIS_BSD) || defined(OPENAIS_DARWIN)
                /* On many OS poll never return POLLHUP or POLLERR.
@@ -638,9 +637,11 @@
                 */
                if (res == 0) {
                        ipc_disconnect (conn_info);
+                       openais_conn_refcount_dec (conn_info);
                        return (0);
                }
 #endif
+               openais_conn_refcount_dec (conn_info);
        }

_______________________________________________
Openais mailing list
Openais@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/openais

Here is a patch to fix a shared memory leak in WhiteTank when used on FreeBSD systems. The problem was that the connection ref counter wasn't decremented as it should when a client disconnected (see patch) and so the shared memory segment was never destroyed by ipc.c:conn_info_destroy(). openais_conn_refcount_inc (conn_info);

smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Openais mailing list
Openais@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/openais