From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=47077 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Phfkh-0006lm-7o
	for qemu-devel@nongnu.org; Tue, 25 Jan 2011 05:03:56 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Phfkf-00034U-KN
	for qemu-devel@nongnu.org; Tue, 25 Jan 2011 05:03:55 -0500
Received: from mail-gw0-f45.google.com ([74.125.83.45]:59230)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Phfkf-00034O-Gh
	for qemu-devel@nongnu.org; Tue, 25 Jan 2011 05:03:53 -0500
Received: by gwaa12 with SMTP id a12so1865507gwa.4
	for <qemu-devel@nongnu.org>; Tue, 25 Jan 2011 02:03:53 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <1295944407-19680-2-git-send-email-corentin.chary@gmail.com>
References: <1295902845-29807-3-git-send-email-aliguori@us.ibm.com>
	<1295944407-19680-2-git-send-email-corentin.chary@gmail.com>
Date: Tue, 25 Jan 2011 10:03:52 +0000
Message-ID: <AANLkTinFNbZwooAX4EVQqeKvmmbC9LhE8n6CaTaiVokK@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH 1/2] vl.c: set NULL upon deleting handlers in
	qemu_set_fd_handler2()
From: Stefan Hajnoczi <stefanha@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Corentin Chary <corentin.chary@gmail.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>, qemu-devel@nongnu.org, Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>

On Tue, Jan 25, 2011 at 8:33 AM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> From: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
>
> Currently qemu_set_fd_handler2() is only setting ioh->deleted upon
> deleting. =A0This may cause a crash when a read handler calls
> qemu_set_fd_handler2() to delete handlers, but a write handler is
> still invoked from main_loop_wait(). =A0Because main_loop_wait() checks
> handlers before calling, setting NULL upon deleting will protect
> handlers being called if already deleted.
>
> One example is the new threaded vnc server. =A0When an error occurs in
> the context of a read handler, it'll releases resources and deletes
> handlers. =A0However, because the write handler still exists, it'll be
> called, and then crashes because of lack of resources. =A0This patch
> fixes it.

Does this case still happen with qemu.git/master?  In November I sent
a patch to check for deleted handlers:

commit 0290b57bdfec83ca78b6d119ea9847bb17943328
Author: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Date:   Wed Nov 3 14:29:44 2010 +0000

    Delete IOHandlers after potentially running them

    Since commit 4bed9837309e58d208183f81d8344996744292cf an .fd_read()
    handler that deletes its IOHandler is exposed to .fd_write() being
    called on the deleted IOHandler.

    This patch fixes deletion so that .fd_read() and .fd_write() are never
    called on an IOHandler that is marked for deletion.

    Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
    Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

So I don't think Yoshi's patch is necessary anymore?

Stefan