From 608593765dcfac22d461bca7081df5e62ea4a02e Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 2 Jul 2020 21:04:56 +0900 Subject: [PATCH 1/9] Don't use PyTuple_Resize It is unsafe. --- MySQLdb/_mysql.c | 66 ++++++++++++++++++------------------------------ 1 file changed, 25 insertions(+), 41 deletions(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index 1556fda3..c45824c2 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1298,19 +1298,16 @@ _mysql_row_to_dict_old( typedef PyObject *_PYFUNC(_mysql_ResultObject *, MYSQL_ROW); -int +Py_ssize_t _mysql__fetch_row( _mysql_ResultObject *self, - PyObject **r, - int skiprows, - int maxrows, + PyObject *r, + Py_ssize_t maxrows, _PYFUNC *convert_row) { - int i; - MYSQL_ROW row; - - for (i = skiprows; i<(skiprows+maxrows); i++) { - PyObject *v; + Py_ssize_t i; + for (i = 0; i < maxrows; i++) { + MYSQL_ROW row; if (!self->use) row = mysql_fetch_row(self->result); else { @@ -1323,14 +1320,17 @@ _mysql__fetch_row( goto error; } if (!row) { - if (_PyTuple_Resize(r, i) == -1) goto error; break; } - v = convert_row(self, row); + PyObject *v = convert_row(self, row); if (!v) goto error; - PyTuple_SET_ITEM(*r, i, v); + if (!PyList_Append(r, v)) { + Py_DECREF(v); + goto error; + } + Py_DECREF(v); } - return i-skiprows; + return i; error: return -1; } @@ -1359,7 +1359,8 @@ _mysql_ResultObject_fetch_row( _mysql_row_to_dict_old }; _PYFUNC *convert_row; - int maxrows=1, how=0, skiprows=0, rowsadded; + int maxrows=1, how=0; + Py_ssize_t rowsadded; PyObject *r=NULL; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ii:fetch_row", kwlist, @@ -1372,39 +1373,22 @@ _mysql_ResultObject_fetch_row( } convert_row = row_converters[how]; if (maxrows) { - if (!(r = PyTuple_New(maxrows))) goto error; - - // see: https://docs.python.org/3/library/gc.html#gc.get_referrers - // This function can get a reference to the tuple r, and if that - // code is preempted while holding a ref to r, the _PyTuple_Resize - // will raise a SystemError because the ref count is 2. - PyObject_GC_UnTrack(r); - rowsadded = _mysql__fetch_row(self, &r, skiprows, maxrows, convert_row); + if (!(r = PyList_New(0))) goto error; + rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); if (rowsadded == -1) goto error; - PyObject_GC_Track(r); } else { if (self->use) { - maxrows = 1000; - if (!(r = PyTuple_New(maxrows))) goto error; - while (1) { - rowsadded = _mysql__fetch_row(self, &r, skiprows, - maxrows, convert_row); - if (rowsadded == -1) goto error; - skiprows += rowsadded; - if (rowsadded < maxrows) break; - if (_PyTuple_Resize(&r, skiprows+maxrows) == -1) - goto error; - } + maxrows = PY_SSIZE_T_MAX; } else { - /* XXX if overflow, maxrows<0? */ - maxrows = (int) mysql_num_rows(self->result); - if (!(r = PyTuple_New(maxrows))) goto error; - rowsadded = _mysql__fetch_row(self, &r, 0, - maxrows, convert_row); - if (rowsadded == -1) goto error; + maxrows = (Py_ssize_t) mysql_num_rows(self->result); } + if (!(r = PyList_New(0))) goto error; + rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); + if (rowsadded == -1) goto error; } - return r; + PyObject *t = PyList_AsTuple(r); + Py_DECREF(r); + return t; error: Py_XDECREF(r); return NULL; From 80fc180f83ed27a7f9bb5b135affd3850e067886 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 2 Jul 2020 21:10:45 +0900 Subject: [PATCH 2/9] cleanup --- MySQLdb/_mysql.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index c45824c2..56fdc56c 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1301,7 +1301,7 @@ typedef PyObject *_PYFUNC(_mysql_ResultObject *, MYSQL_ROW); Py_ssize_t _mysql__fetch_row( _mysql_ResultObject *self, - PyObject *r, + PyObject *r, /* list object */ Py_ssize_t maxrows, _PYFUNC *convert_row) { @@ -1317,22 +1317,20 @@ _mysql__fetch_row( } if (!row && mysql_errno(&(((_mysql_ConnectionObject *)(self->conn))->connection))) { _mysql_Exception((_mysql_ConnectionObject *)self->conn); - goto error; + return -1; } if (!row) { break; } PyObject *v = convert_row(self, row); - if (!v) goto error; + if (!v) return -1; if (!PyList_Append(r, v)) { Py_DECREF(v); - goto error; + return -1; } Py_DECREF(v); } return i; - error: - return -1; } static char _mysql_ResultObject_fetch_row__doc__[] = @@ -1380,15 +1378,22 @@ _mysql_ResultObject_fetch_row( if (self->use) { maxrows = PY_SSIZE_T_MAX; } else { + // todo: preallocate. maxrows = (Py_ssize_t) mysql_num_rows(self->result); } if (!(r = PyList_New(0))) goto error; rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); if (rowsadded == -1) goto error; } - PyObject *t = PyList_AsTuple(r); + /* DB-API allows return rows as list. + * But since Django tests return value with (), we need to return empty + * tuple instead of empty list. + */ + if (PyList_Size(r) > 0) { + return r; + } Py_DECREF(r); - return t; + return PyTuple_New(0); error: Py_XDECREF(r); return NULL; From f687bec212477ae16e79e6815efa0d825f98ec4a Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 2 Jul 2020 21:13:08 +0900 Subject: [PATCH 3/9] fix --- MySQLdb/_mysql.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index 56fdc56c..fba99e49 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1324,7 +1324,7 @@ _mysql__fetch_row( } PyObject *v = convert_row(self, row); if (!v) return -1; - if (!PyList_Append(r, v)) { + if (PyList_Append(r, v)) { Py_DECREF(v); return -1; } @@ -1389,7 +1389,7 @@ _mysql_ResultObject_fetch_row( * But since Django tests return value with (), we need to return empty * tuple instead of empty list. */ - if (PyList_Size(r) > 0) { + if (rowsadded > 0) { return r; } Py_DECREF(r); From 9aa5633c2f3a6534eb6f4a4c817c470b9c8d2536 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 2 Jul 2020 22:57:21 +0900 Subject: [PATCH 4/9] maxrows is int --- MySQLdb/_mysql.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index fba99e49..a9c075f3 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1370,21 +1370,18 @@ _mysql_ResultObject_fetch_row( return NULL; } convert_row = row_converters[how]; - if (maxrows) { - if (!(r = PyList_New(0))) goto error; - rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); - if (rowsadded == -1) goto error; - } else { + if (!maxrows) { if (self->use) { - maxrows = PY_SSIZE_T_MAX; + maxrows = INT_MAX; } else { // todo: preallocate. maxrows = (Py_ssize_t) mysql_num_rows(self->result); } - if (!(r = PyList_New(0))) goto error; - rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); - if (rowsadded == -1) goto error; } + if (!(r = PyList_New(0))) goto error; + rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); + if (rowsadded == -1) goto error; + /* DB-API allows return rows as list. * But since Django tests return value with (), we need to return empty * tuple instead of empty list. From ee892142bcf5c33d75a5f45482c72ae6fe7706c2 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 2 Jul 2020 23:33:56 +0900 Subject: [PATCH 5/9] Use Python 3.8 in Django test --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2f956c2d..7087399d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,7 +42,7 @@ jobs: name: "Django 2.2 test" env: - DJANGO_VERSION=2.2.7 - python: "3.5" + python: "3.8" install: - pip install -U pip - wget https://github.com/django/django/archive/${DJANGO_VERSION}.tar.gz From 2b9c88b3bd3f43a14fe5685ecc33020d5f2e2ddd Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Fri, 3 Jul 2020 01:05:24 +0900 Subject: [PATCH 6/9] mallocstats --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 7087399d..b98fcfc6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -58,7 +58,9 @@ jobs: script: - cd django-${DJANGO_VERSION}/tests/ - - ./runtests.py --parallel=1 --settings=test_mysql + - free -m + - PYTHONMALLOCSTATS=1 ./runtests.py --parallel=1 --settings=test_mysql + - free -m - name: flake8 python: "3.8" install: From d197010b6ec481670c1b468d156819f0c9268fbc Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Fri, 3 Jul 2020 08:54:18 +0900 Subject: [PATCH 7/9] no stats --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index b98fcfc6..d9bfc5ab 100644 --- a/.travis.yml +++ b/.travis.yml @@ -59,7 +59,7 @@ jobs: script: - cd django-${DJANGO_VERSION}/tests/ - free -m - - PYTHONMALLOCSTATS=1 ./runtests.py --parallel=1 --settings=test_mysql + - /usr/bin/time ./runtests.py --parallel=1 --settings=test_mysql - free -m - name: flake8 python: "3.8" From 5c447625b0458940e8e74be2f098f832ee1f9807 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Fri, 3 Jul 2020 10:16:40 +0900 Subject: [PATCH 8/9] Return tuple --- .travis.yml | 7 +++---- MySQLdb/_mysql.c | 12 ++++-------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/.travis.yml b/.travis.yml index d9bfc5ab..af7b7f6f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ language: python python: - "nightly" - "pypy3" + - "3.9-dev" - "3.8" - "3.7" - "3.6" @@ -41,7 +42,7 @@ jobs: - &django_2_2 name: "Django 2.2 test" env: - - DJANGO_VERSION=2.2.7 + - DJANGO_VERSION=2.2.14 python: "3.8" install: - pip install -U pip @@ -58,9 +59,7 @@ jobs: script: - cd django-${DJANGO_VERSION}/tests/ - - free -m - - /usr/bin/time ./runtests.py --parallel=1 --settings=test_mysql - - free -m + - ./runtests.py --parallel=1 --settings=test_mysql - name: flake8 python: "3.8" install: diff --git a/MySQLdb/_mysql.c b/MySQLdb/_mysql.c index a9c075f3..62d0969d 100644 --- a/MySQLdb/_mysql.c +++ b/MySQLdb/_mysql.c @@ -1358,7 +1358,6 @@ _mysql_ResultObject_fetch_row( }; _PYFUNC *convert_row; int maxrows=1, how=0; - Py_ssize_t rowsadded; PyObject *r=NULL; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ii:fetch_row", kwlist, @@ -1379,18 +1378,15 @@ _mysql_ResultObject_fetch_row( } } if (!(r = PyList_New(0))) goto error; - rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); + Py_ssize_t rowsadded = _mysql__fetch_row(self, r, maxrows, convert_row); if (rowsadded == -1) goto error; /* DB-API allows return rows as list. - * But since Django tests return value with (), we need to return empty - * tuple instead of empty list. + * But we need to return list because Django expecting tuple. */ - if (rowsadded > 0) { - return r; - } + PyObject *t = PyList_AsTuple(r); Py_DECREF(r); - return PyTuple_New(0); + return t; error: Py_XDECREF(r); return NULL; From 52bd5132f4f703c1f320c8b358fcb56ae9ce46b4 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Fri, 3 Jul 2020 10:41:25 +0900 Subject: [PATCH 9/9] Django 2.2.7 --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index af7b7f6f..ec1cd379 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,7 +42,7 @@ jobs: - &django_2_2 name: "Django 2.2 test" env: - - DJANGO_VERSION=2.2.14 + - DJANGO_VERSION=2.2.7 python: "3.8" install: - pip install -U pip @@ -59,7 +59,7 @@ jobs: script: - cd django-${DJANGO_VERSION}/tests/ - - ./runtests.py --parallel=1 --settings=test_mysql + - ./runtests.py --parallel=2 --settings=test_mysql - name: flake8 python: "3.8" install: