[Python-Dev] Segfault

[Hrvoje Nikšić]

On Fri, 2007-08-24 at 09:01 -0700, Neal Norwitz wrote:
> Right.  I looked at this with Jeffrey Yasskin and agree that a lock is
> needed for f_fp.  The fix is ugly because it's needed around all
> accesses to f_fp and there are a ton of them.  Some are with the GIL
> held and others when it isn't.

The way I see it, when the GIL is held, we're in the clear because
f->f_fp can only disappear with the GIL held.  The troublesome cases are
those ones where f_fp is accessed inside an "allow threads" section.

To fix this, it shouldn't be necessary to change every individual place
that accesses f_fp.  It is sufficient to protect the sections that
specifically allow threads, changing the semantics from "allow threads"
to "allow threads, locking out the ones operating on the same
fileobject".

For example, instead of Py_BEGIN_ALLOW_THREADS, the fileobject code
could use a macro such as:

#define PyFile_BEGIN_PROTECT(f)          \
  PyThread_acquire_lock(f->f_lock, 1);   \
  if (!f->f_fp)                          \
    ;                                    \
  else {

#define PyFile_END_PROTECT(f)            \
  }                                      \
  PyThread_release_lock(f->f_lock);

That change might be somewhat easier than changing each individual line
that accesses f->f_fp.

> I would be fine with the much simpler approach in the patch I sent
> (assuming the patch is finished).  It doesn't completely address the
> problem, but does make the race condition much, much smaller.  Part of
> the reason I'm ok with this is that in 3.0, all this code has already
> been removed.

That is a good point.  Does your patch make the race crash go away
entirely in the tests like the one the OP posted?