Date: Wed, 09 Jun 2004 18:20:00 +0200 From: "Poul-Henning Kamp" <phk@phk.freebsd.dk> To: "M. Warner Losh" <imp@bsdimp.com> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern kern_proc.c Message-ID: <55929.1086798000@critter.freebsd.dk> In-Reply-To: Your message of "Wed, 09 Jun 2004 10:04:13 MDT." <20040609.100413.118633043.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <20040609.100413.118633043.imp@bsdimp.com>, "M. Warner Losh" writes: >Can you provide a couple of lines about why BAD is BAD and why GOOD >fixes that flaw? That should help others from making this mistake in >the future. LOCK(foo->lock) foo->refcount--; UNLOCK(foo->lock) if (foo->refcount == 0) destroy(foo); The problem is that there is the risk that another thread will modify the refcount between our modification and our test: Assume foo->refcount = 2; thread1 (low priority) thread2 (high priority) ---------------------- ----------------------- ... ... LOCK(foo->lock) ... foo->refcount--; ... # refcount now == 1 LOCK(foo->lock) At this point, thread2 sleeps, spins or whatever until it can get the lock it wants. UNLOCK(foo->lock) Now thread2 is runnable and since it has a higher priority it will be run: foo->refcount--; # refcount now == 0 UNLOCK(foo->lock); if(foo->refount == 0) destroy(foo); ... At some point thread1 gets to continue: if (foo->refcount == 0) destroy(foo); But at this time foo may be gone or recycled and a panic is our best hope and random memory corruption is our worst fear. The way to fix this is to make sure that the test for zero-ness is done on the result of our own decrement operation: LOCK(foo->lock) i = --foo->refcount; UNLOCK(foo->lock) if (i == 0) destroy(foo); Assume foo->refcount = 2; thread1 (low priority) thread2 (high priority) ---------------------- ----------------------- ... ... LOCK(foo->lock) ... i = --foo->refcount; LOCK(foo->lock) # i == 1, refcount == 1 UNLOCK(foo->lock) i = --foo->refcount; # i == 0, refcount == 0 UNLOCK(foo->lock) if (i == 0) # true destroy(foo) ... if (i == 0) # false destroy(foo) I'm not very good at explaining this am I ? -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 phk@FreeBSD.ORG | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55929.1086798000>