Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(49002)

Delta Between Two Patch Sets: Modules/posixmodule.c

Issue 25994: File descriptor leaks in os.scandir()
Left Patch Set: Created 3 years, 7 months ago
Right Patch Set: Created 3 years, 7 months ago
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« Lib/test/test_os.py ('K') | « Misc/NEWS ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 1
2 /* POSIX module implementation */ 2 /* POSIX module implementation */
3 3
4 /* This file is also used for Windows NT/MS-Win. In that case the 4 /* This file is also used for Windows NT/MS-Win. In that case the
5 module actually calls itself 'nt', not 'posix', and a few 5 module actually calls itself 'nt', not 'posix', and a few
6 functions are either unimplemented or implemented differently. The source 6 functions are either unimplemented or implemented differently. The source
7 assumes that for Windows NT, the macro 'MS_WINDOWS' is defined independent 7 assumes that for Windows NT, the macro 'MS_WINDOWS' is defined independent
8 of the compiler used. Different compilers define their own feature 8 of the compiler used. Different compilers define their own feature
9 test macro, e.g. '_MSC_VER'. */ 9 test macro, e.g. '_MSC_VER'. */
10 10
(...skipping 11919 matching lines...) Expand 10 before | Expand all | Expand 10 after
11930 HANDLE handle; 11930 HANDLE handle;
11931 WIN32_FIND_DATAW file_data; 11931 WIN32_FIND_DATAW file_data;
11932 int first_time; 11932 int first_time;
11933 #else /* POSIX */ 11933 #else /* POSIX */
11934 DIR *dirp; 11934 DIR *dirp;
11935 #endif 11935 #endif
11936 } ScandirIterator; 11936 } ScandirIterator;
11937 11937
11938 #ifdef MS_WINDOWS 11938 #ifdef MS_WINDOWS
11939 11939
11940 static int
11941 ScandirIterator_is_closed(ScandirIterator *iterator)
11942 {
11943 return iterator->handle == INVALID_HANDLE_VALUE;
11944 }
11945
11940 static void 11946 static void
11941 ScandirIterator_closedir(ScandirIterator *iterator) 11947 ScandirIterator_closedir(ScandirIterator *iterator)
11942 { 11948 {
11943 if (iterator->handle == INVALID_HANDLE_VALUE) 11949 if (iterator->handle == INVALID_HANDLE_VALUE)
11944 return; 11950 return;
11945 11951
11946 Py_BEGIN_ALLOW_THREADS 11952 Py_BEGIN_ALLOW_THREADS
11947 FindClose(iterator->handle); 11953 FindClose(iterator->handle);
11948 Py_END_ALLOW_THREADS 11954 Py_END_ALLOW_THREADS
11949 iterator->handle = INVALID_HANDLE_VALUE; 11955 iterator->handle = INVALID_HANDLE_VALUE;
11950 } 11956 }
11951 11957
11952 static PyObject * 11958 static PyObject *
11953 ScandirIterator_iternext(ScandirIterator *iterator) 11959 ScandirIterator_iternext(ScandirIterator *iterator)
11954 { 11960 {
11955 WIN32_FIND_DATAW *file_data = &iterator->file_data; 11961 WIN32_FIND_DATAW *file_data = &iterator->file_data;
11956 BOOL success; 11962 BOOL success;
11957 PyObject *entry; 11963 PyObject *entry;
11958 11964
11959 /* Happens if the iterator is iterated twice */ 11965 /* Happens if the iterator is iterated twice, or closed explicitly */
haypo 2016/02/08 23:12:44 You should update the comment. Something like: /*
storchaka 2016/02/09 16:11:45 Done.
11960 if (iterator->handle == INVALID_HANDLE_VALUE) 11966 if (iterator->handle == INVALID_HANDLE_VALUE)
11961 return NULL; 11967 return NULL;
11962 11968
11963 while (1) { 11969 while (1) {
11964 if (!iterator->first_time) { 11970 if (!iterator->first_time) {
11965 Py_BEGIN_ALLOW_THREADS 11971 Py_BEGIN_ALLOW_THREADS
11966 success = FindNextFileW(iterator->handle, file_data); 11972 success = FindNextFileW(iterator->handle, file_data);
11967 Py_END_ALLOW_THREADS 11973 Py_END_ALLOW_THREADS
11968 if (!success) { 11974 if (!success) {
11969 /* Error or no more files */ 11975 /* Error or no more files */
(...skipping 16 matching lines...) Expand all
11986 /* Loop till we get a non-dot directory or finish iterating */ 11992 /* Loop till we get a non-dot directory or finish iterating */
11987 } 11993 }
11988 11994
11989 /* Error or no more files */ 11995 /* Error or no more files */
11990 ScandirIterator_closedir(iterator); 11996 ScandirIterator_closedir(iterator);
11991 return NULL; 11997 return NULL;
11992 } 11998 }
11993 11999
11994 #else /* POSIX */ 12000 #else /* POSIX */
11995 12001
12002 static int
12003 ScandirIterator_is_closed(ScandirIterator *iterator)
12004 {
12005 return !iterator->dirp;
12006 }
12007
11996 static void 12008 static void
11997 ScandirIterator_closedir(ScandirIterator *iterator) 12009 ScandirIterator_closedir(ScandirIterator *iterator)
11998 { 12010 {
11999 if (!iterator->dirp) 12011 if (!iterator->dirp)
12000 return; 12012 return;
12001 12013
12002 Py_BEGIN_ALLOW_THREADS 12014 Py_BEGIN_ALLOW_THREADS
12003 closedir(iterator->dirp); 12015 closedir(iterator->dirp);
12004 Py_END_ALLOW_THREADS 12016 Py_END_ALLOW_THREADS
12005 iterator->dirp = NULL; 12017 iterator->dirp = NULL;
12006 return; 12018 return;
12007 } 12019 }
12008 12020
12009 static PyObject * 12021 static PyObject *
12010 ScandirIterator_iternext(ScandirIterator *iterator) 12022 ScandirIterator_iternext(ScandirIterator *iterator)
12011 { 12023 {
12012 struct dirent *direntp; 12024 struct dirent *direntp;
12013 Py_ssize_t name_len; 12025 Py_ssize_t name_len;
12014 int is_dot; 12026 int is_dot;
12015 PyObject *entry; 12027 PyObject *entry;
12016 12028
12017 /* Happens if the iterator is iterated twice */ 12029 /* Happens if the iterator is iterated twice, or closed explicitly */
haypo 2016/02/08 23:12:44 ditto
storchaka 2016/02/09 16:11:45 Done.
12018 if (!iterator->dirp) 12030 if (!iterator->dirp)
12019 return NULL; 12031 return NULL;
12020 12032
12021 while (1) { 12033 while (1) {
12022 errno = 0; 12034 errno = 0;
12023 Py_BEGIN_ALLOW_THREADS 12035 Py_BEGIN_ALLOW_THREADS
12024 direntp = readdir(iterator->dirp); 12036 direntp = readdir(iterator->dirp);
12025 Py_END_ALLOW_THREADS 12037 Py_END_ALLOW_THREADS
12026 12038
12027 if (!direntp) { 12039 if (!direntp) {
(...skipping 46 matching lines...) Expand 10 before | Expand all | Expand 10 after
12074 static PyObject * 12086 static PyObject *
12075 ScandirIterator_exit(ScandirIterator *self, PyObject *args) 12087 ScandirIterator_exit(ScandirIterator *self, PyObject *args)
12076 { 12088 {
12077 ScandirIterator_closedir(self); 12089 ScandirIterator_closedir(self);
12078 Py_RETURN_NONE; 12090 Py_RETURN_NONE;
12079 } 12091 }
12080 12092
12081 static void 12093 static void
12082 ScandirIterator_dealloc(ScandirIterator *iterator) 12094 ScandirIterator_dealloc(ScandirIterator *iterator)
12083 { 12095 {
12084 ScandirIterator_closedir(iterator); 12096 if (!ScandirIterator_is_closed(iterator)) {
12097 PyObject *exc, *val, *tb;
12098 Py_ssize_t old_refcount = Py_REFCNT(iterator);
12099 /* Py_INCREF/Py_DECREF cannot be used, because the refcount is
12100 * likely zero, Py_DECREF would call again the destructor.
12101 */
12102 ++Py_REFCNT(iterator);
12103 PyErr_Fetch(&exc, &val, &tb);
12104 if (PyErr_WarnFormat(PyExc_ResourceWarning, 1,
12105 "unclosed scandir iterator %R", iterator)) {
12106 /* Spurious errors can appear at shutdown */
12107 if (PyErr_ExceptionMatches(PyExc_Warning))
12108 PyErr_WriteUnraisable((PyObject *) iterator);
12109 }
12110 PyErr_Restore(exc, val, tb);
12111 Py_REFCNT(iterator) = old_refcount;
12112
12113 ScandirIterator_closedir(iterator);
12114 }
12085 Py_XDECREF(iterator->path.object); 12115 Py_XDECREF(iterator->path.object);
12086 path_cleanup(&iterator->path); 12116 path_cleanup(&iterator->path);
12087 Py_TYPE(iterator)->tp_free((PyObject *)iterator); 12117 Py_TYPE(iterator)->tp_free((PyObject *)iterator);
12088 } 12118 }
12089 12119
12090 static PyMethodDef ScandirIterator_methods[] = { 12120 static PyMethodDef ScandirIterator_methods[] = {
12091 {"__enter__", (PyCFunction)ScandirIterator_enter, METH_NOARGS}, 12121 {"__enter__", (PyCFunction)ScandirIterator_enter, METH_NOARGS},
12092 {"__exit__", (PyCFunction)ScandirIterator_exit, METH_VARARGS}, 12122 {"__exit__", (PyCFunction)ScandirIterator_exit, METH_VARARGS},
12093 {"close", (PyCFunction)ScandirIterator_close, METH_NOARGS}, 12123 {"close", (PyCFunction)ScandirIterator_close, METH_NOARGS},
haypo 2016/02/08 23:12:44 I tried your patch. It took me 10 min to understan
storchaka 2016/02/09 16:11:45 Good catch!
12124 {NULL}
12094 }; 12125 };
12095 12126
12096 static PyTypeObject ScandirIteratorType = { 12127 static PyTypeObject ScandirIteratorType = {
12097 PyVarObject_HEAD_INIT(NULL, 0) 12128 PyVarObject_HEAD_INIT(NULL, 0)
12098 MODNAME ".ScandirIterator", /* tp_name */ 12129 MODNAME ".ScandirIterator", /* tp_name */
12099 sizeof(ScandirIterator), /* tp_basicsize */ 12130 sizeof(ScandirIterator), /* tp_basicsize */
12100 0, /* tp_itemsize */ 12131 0, /* tp_itemsize */
12101 /* methods */ 12132 /* methods */
12102 (destructor)ScandirIterator_dealloc, /* tp_dealloc */ 12133 (destructor)ScandirIterator_dealloc, /* tp_dealloc */
12103 0, /* tp_print */ 12134 0, /* tp_print */
(...skipping 1067 matching lines...) Expand 10 before | Expand all | Expand 10 after
13171 PyModule_AddObject(m, "_have_functions", list); 13202 PyModule_AddObject(m, "_have_functions", list);
13172 13203
13173 initialized = 1; 13204 initialized = 1;
13174 13205
13175 return m; 13206 return m;
13176 } 13207 }
13177 13208
13178 #ifdef __cplusplus 13209 #ifdef __cplusplus
13179 } 13210 }
13180 #endif 13211 #endif
LEFTRIGHT
« Misc/NEWS ('k') | no next file » | Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Toggle Comments ('s')

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+