On 8/22/07, Hrvoje Nikšić <[EMAIL PROTECTED]> wrote:
> On Tue, 2007-08-21 at 09:14 -0700, Neal Norwitz wrote:
> > The patch is insufficient to prevent all types of crashes that occur
> > when accessing a file from 2 threads (closing in one and doing
> > whatever in another).
>
> You are right. I wouldn't go so far to say the file object
> thread-unsafe, but it certainly has lurking bugs with calling close
> while other threads are running. BTW your mail doesn't seem to contain
> the actual patch.
Sorry about that. I thought something was up when gmail didn't
automatically upload it while I was writing the message. It looks
like it took this time. (The attached patch isn't complete, but
catches most cases.)
n
Index: Objects/fileobject.c
===================================================================
--- Objects/fileobject.c (revision 57244)
+++ Objects/fileobject.c (working copy)
@@ -439,13 +439,16 @@
{
int sts = 0;
if (f->f_fp != NULL) {
+ /* Invalidate f->f_fp *before* closing, so we don't attempt
+ to close it twice in separate threads. */
+ FILE *tmp_fp = f->f_fp;
+ f->f_fp = NULL;
if (f->f_close != NULL) {
Py_BEGIN_ALLOW_THREADS
errno = 0;
- sts = (*f->f_close)(f->f_fp);
+ sts = (*f->f_close)(tmp_fp);
Py_END_ALLOW_THREADS
}
- f->f_fp = NULL;
}
PyMem_Free(f->f_setbuf);
f->f_setbuf = NULL;
@@ -539,7 +542,7 @@
file_seek(PyFileObject *f, PyObject *args)
{
int whence;
- int ret;
+ int ret = 0;
Py_off_t offset;
PyObject *offobj;
@@ -560,8 +563,11 @@
Py_BEGIN_ALLOW_THREADS
errno = 0;
- ret = _portable_fseek(f->f_fp, offset, whence);
+ if (f->f_fp != NULL)
+ ret = _portable_fseek(f->f_fp, offset, whence);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (ret != 0) {
PyErr_SetFromErrno(PyExc_IOError);
@@ -580,8 +586,8 @@
{
Py_off_t newsize;
PyObject *newsizeobj = NULL;
- Py_off_t initialpos;
- int ret;
+ Py_off_t initialpos = 0;
+ int ret = 0;
if (f->f_fp == NULL)
return err_closed();
@@ -597,8 +603,11 @@
*/
Py_BEGIN_ALLOW_THREADS
errno = 0;
- initialpos = _portable_ftell(f->f_fp);
+ if (f->f_fp != NULL)
+ initialpos = _portable_ftell(f->f_fp);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (initialpos == -1)
goto onioerror;
@@ -625,8 +634,11 @@
*/
Py_BEGIN_ALLOW_THREADS
errno = 0;
- ret = fflush(f->f_fp);
+ if (f->f_fp != NULL)
+ ret = fflush(f->f_fp);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (ret != 0)
goto onioerror;
@@ -639,30 +651,40 @@
/* Have to move current pos to desired endpoint on Windows. */
Py_BEGIN_ALLOW_THREADS
errno = 0;
- ret = _portable_fseek(f->f_fp, newsize, SEEK_SET) != 0;
+ if (f->f_fp != NULL)
+ ret = _portable_fseek(f->f_fp, newsize, SEEK_SET) != 0;
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (ret)
goto onioerror;
/* Truncate. Note that this may grow the file! */
Py_BEGIN_ALLOW_THREADS
errno = 0;
- hFile = (HANDLE)_get_osfhandle(fileno(f->f_fp));
- ret = hFile == (HANDLE)-1;
- if (ret == 0) {
- ret = SetEndOfFile(hFile) == 0;
- if (ret)
- errno = EACCES;
+ if (f->f_fp != NULL) {
+ hFile = (HANDLE)_get_osfhandle(fileno(f->f_fp));
+ ret = hFile == (HANDLE)-1;
+ if (ret == 0) {
+ ret = SetEndOfFile(hFile) == 0;
+ if (ret)
+ errno = EACCES;
+ }
}
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (ret)
goto onioerror;
}
#else
Py_BEGIN_ALLOW_THREADS
errno = 0;
- ret = ftruncate(fileno(f->f_fp), newsize);
+ if (f->f_fp != NULL)
+ ret = ftruncate(fileno(f->f_fp), newsize);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (ret != 0)
goto onioerror;
#endif /* !MS_WINDOWS */
@@ -670,8 +692,11 @@
/* Restore original file position. */
Py_BEGIN_ALLOW_THREADS
errno = 0;
- ret = _portable_fseek(f->f_fp, initialpos, SEEK_SET) != 0;
+ if (f->f_fp != NULL)
+ ret = _portable_fseek(f->f_fp, initialpos, SEEK_SET) != 0;
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (ret)
goto onioerror;
@@ -688,14 +713,17 @@
static PyObject *
file_tell(PyFileObject *f)
{
- Py_off_t pos;
+ Py_off_t pos = 0;
if (f->f_fp == NULL)
return err_closed();
Py_BEGIN_ALLOW_THREADS
errno = 0;
- pos = _portable_ftell(f->f_fp);
+ if (f->f_fp != NULL)
+ pos = _portable_ftell(f->f_fp);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (pos == -1) {
PyErr_SetFromErrno(PyExc_IOError);
clearerr(f->f_fp);
@@ -727,14 +755,17 @@
static PyObject *
file_flush(PyFileObject *f)
{
- int res;
+ int res = 0;
if (f->f_fp == NULL)
return err_closed();
Py_BEGIN_ALLOW_THREADS
errno = 0;
- res = fflush(f->f_fp);
+ if (f->f_fp != NULL)
+ res = fflush(f->f_fp);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (res != 0) {
PyErr_SetFromErrno(PyExc_IOError);
clearerr(f->f_fp);
@@ -747,12 +778,15 @@
static PyObject *
file_isatty(PyFileObject *f)
{
- long res;
+ long res = 0;
if (f->f_fp == NULL)
return err_closed();
Py_BEGIN_ALLOW_THREADS
- res = isatty((int)fileno(f->f_fp));
+ if (f->f_fp != NULL)
+ res = isatty((int)fileno(f->f_fp));
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
return PyBool_FromLong(res);
}
@@ -826,7 +860,7 @@
file_read(PyFileObject *f, PyObject *args)
{
long bytesrequested = -1;
- size_t bytesread, buffersize, chunksize;
+ size_t bytesread, buffersize, chunksize = 0;
PyObject *v;
if (f->f_fp == NULL)
@@ -854,9 +888,14 @@
for (;;) {
Py_BEGIN_ALLOW_THREADS
errno = 0;
- chunksize = Py_UniversalNewlineFread(BUF(v) + bytesread,
- buffersize - bytesread, f->f_fp, (PyObject *)f);
+ if (f->f_fp != NULL)
+ chunksize = Py_UniversalNewlineFread(BUF(v) + bytesread,
+ buffersize - bytesread, f->f_fp, (PyObject *)f);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL) {
+ Py_DECREF(v);
+ return err_closed();
+ }
if (chunksize == 0) {
if (!ferror(f->f_fp))
break;
@@ -895,7 +934,7 @@
{
char *ptr;
Py_ssize_t ntodo;
- Py_ssize_t ndone, nnow;
+ Py_ssize_t ndone, nnow = 0;
if (f->f_fp == NULL)
return err_closed();
@@ -910,9 +949,13 @@
while (ntodo > 0) {
Py_BEGIN_ALLOW_THREADS
errno = 0;
- nnow = Py_UniversalNewlineFread(ptr+ndone, ntodo, f->f_fp,
+ if (f->f_fp != NULL)
+ nnow = Py_UniversalNewlineFread(ptr+ndone, ntodo,
+ f->f_fp,
(PyObject *)f);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (nnow == 0) {
if (!ferror(f->f_fp))
break;
@@ -1379,7 +1422,7 @@
size_t buffersize = SMALLCHUNK;
PyObject *big_buffer = NULL;
size_t nfilled = 0;
- size_t nread;
+ size_t nread = 0;
size_t totalread = 0;
char *p, *q, *end;
int err;
@@ -1402,9 +1445,13 @@
else {
Py_BEGIN_ALLOW_THREADS
errno = 0;
- nread = Py_UniversalNewlineFread(buffer+nfilled,
- buffersize-nfilled, f->f_fp, (PyObject *)f);
+ if (f->f_fp != NULL)
+ nread = Py_UniversalNewlineFread(buffer+nfilled,
+ buffersize-nfilled,
+ f->f_fp, (PyObject *)f);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
shortread = (nread < buffersize-nfilled);
}
if (nread == 0) {
@@ -1499,7 +1546,7 @@
file_write(PyFileObject *f, PyObject *args)
{
char *s;
- Py_ssize_t n, n2;
+ Py_ssize_t n, n2 = 0;
if (f->f_fp == NULL)
return err_closed();
if (!PyArg_ParseTuple(args, f->f_binary ? "s#" : "t#", &s, &n))
@@ -1507,8 +1554,11 @@
f->f_softspace = 0;
Py_BEGIN_ALLOW_THREADS
errno = 0;
- n2 = fwrite(s, 1, n, f->f_fp);
+ if (f->f_fp != NULL)
+ n2 = fwrite(s, 1, n, f->f_fp);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL)
+ return err_closed();
if (n2 != n) {
PyErr_SetFromErrno(PyExc_IOError);
clearerr(f->f_fp);
@@ -1842,7 +1892,7 @@
static int
readahead(PyFileObject *f, int bufsize)
{
- Py_ssize_t chunksize;
+ Py_ssize_t chunksize = 0;
if (f->f_buf != NULL) {
if( (f->f_bufend - f->f_bufptr) >= 1)
@@ -1856,9 +1906,15 @@
}
Py_BEGIN_ALLOW_THREADS
errno = 0;
- chunksize = Py_UniversalNewlineFread(
- f->f_buf, bufsize, f->f_fp, (PyObject *)f);
+ if (f->f_fp != NULL)
+ chunksize = Py_UniversalNewlineFread(
+ f->f_buf, bufsize, f->f_fp, (PyObject *)f);
Py_END_ALLOW_THREADS
+ if (f->f_fp == NULL) {
+ err_closed();
+ drop_readahead(f);
+ return -1;
+ }
if (chunksize == 0) {
if (ferror(f->f_fp)) {
PyErr_SetFromErrno(PyExc_IOError);
_______________________________________________
Python-Dev mailing list
[email protected]
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
