Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: korablev@tarantool.org, Mergen Imeev <imeevma@gmail.com>
Subject: [tarantool-patches] [PATCH 3/3] sql: too many autogenerated ids leads to SEGFAULT
Date: Mon, 19 Nov 2018 16:47:30 +0300	[thread overview]
Message-ID: <6128bdce00cc4c6d47c122897c195e30e0416613.1542635129.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1542635129.git.v.shpilevoy@tarantool.org>
In-Reply-To: <cover.1542635129.git.v.shpilevoy@tarantool.org>

From: Mergen Imeev <imeevma@gmail.com>

This probleam appeared because region was cleaned twice: once in
sqlite3VdbeHalt() and once in sqlite3VdbeDelete() which was
executed during sqlite3_finalize(). Autogenerated ids that were
saved there, were fetched after sqlite3VdbeHalt() and before
sqlite3_finalize(). In this patch region cleaning in
sqlite3VdbeHalt() were removed.

Follow up #2618
Follow up #3199
---
 src/box/sql/vdbe.c       |  8 ++------
 src/box/sql/vdbeaux.c    |  6 ------
 test/sql/iproto.result   | 16 ++++++++++++++++
 test/sql/iproto.test.lua |  7 +++++++
 4 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index b6afe9184..cc340e942 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2911,12 +2911,8 @@ case OP_MakeRecord: {
 	 * memory shouldn't be reused until it is written into WAL.
 	 *
 	 * However, if memory for ephemeral space is allocated
-	 * on region, it will be freed only in vdbeHalt() routine.
-	 * It is the only way to free this region memory,
-	 * since ephemeral spaces don't have nothing in common
-	 * with txn routine and region memory won't be released
-	 * after txn_commit() or txn_rollback() as it happens
-	 * with ordinary spaces.
+	 * on region, it will be freed only in sqlite3_finalize()
+	 * routine.
 	 */
 	if (bIsEphemeral) {
 		rc = sqlite3VdbeMemClearAndResize(pOut, nByte);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 615a0f064..f2faad862 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2498,12 +2498,6 @@ sqlite3VdbeHalt(Vdbe * p)
 		p->rc = SQLITE_NOMEM_BKPT;
 	}
 
-	/* Release all region memory which was allocated
-	 * to hold tuples to be inserted into ephemeral spaces.
-	 */
-	if (!box_txn())
-		fiber_gc();
-
 	assert(db->nVdbeActive > 0 || box_txn() ||
 	       p->anonymous_savepoint == NULL);
 	return (p->rc == SQLITE_BUSY ? SQLITE_BUSY : SQLITE_OK);
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index b1313e10e..b2a1e42cc 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -794,6 +794,22 @@ res.rows
 - - [[{'name': 'space_id', 'type': 'unsigned'}, {'name': 'lsn', 'type': 'unsigned'},
       {'name': 'tuple', 'type': 'array'}]]
 ...
+-- Too many autogenerated ids leads to SEGFAULT.
+cn = remote.connect(box.cfg.listen)
+---
+...
+box.sql.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY AUTOINCREMENT)')
+---
+...
+for i = 0, 1000 do cn:execute("INSERT INTO t1 VALUES (null)") end
+---
+...
+_ = cn:execute("INSERT INTO t1 SELECT NULL from t1")
+---
+...
+box.sql.execute('DROP TABLE t1')
+---
+...
 cn:close()
 ---
 ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 5c90cba52..da52d631e 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -261,6 +261,13 @@ _ = cn:execute("EXPLAIN SELECT 1;")
 res = cn:execute('select "format" from "_space" limit 1;')
 res.rows
 
+-- Too many autogenerated ids leads to SEGFAULT.
+cn = remote.connect(box.cfg.listen)
+box.sql.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY AUTOINCREMENT)')
+for i = 0, 1000 do cn:execute("INSERT INTO t1 VALUES (null)") end
+_ = cn:execute("INSERT INTO t1 SELECT NULL from t1")
+box.sql.execute('DROP TABLE t1')
+
 cn:close()
 
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
-- 
2.17.2 (Apple Git-113)

  parent reply	other threads:[~2018-11-19 13:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-19 13:47 [tarantool-patches] [PATCH 0/3] A set of small SQL fixes Vladislav Shpilevoy
2018-11-19 13:47 ` [tarantool-patches] [PATCH 1/3] sql: EXPLAIN through net.box leads to SEGFAULT Vladislav Shpilevoy
2018-11-19 17:27   ` [tarantool-patches] " n.pettik
2018-11-22 18:08     ` Imeev Mergen
2018-11-22 18:31       ` Vladislav Shpilevoy
2018-11-22 20:55         ` [tarantool-patches] Re[2]: " Мерген Имеев
2018-11-23  6:04           ` [tarantool-patches] Re: Re[2]: " Kirill Yukhin
2018-11-28 12:36           ` [tarantool-patches] " n.pettik
2018-11-29 14:58             ` Konstantin Osipov
2018-11-19 13:47 ` [tarantool-patches] [PATCH 2/3] sql: SELECT from system spaces returns unpacked msgpack Vladislav Shpilevoy
2018-11-19 17:27   ` [tarantool-patches] " n.pettik
2018-11-22 18:09     ` Imeev Mergen
2018-11-27  8:51       ` n.pettik
2018-11-19 13:47 ` Vladislav Shpilevoy [this message]
2018-11-19 17:27   ` [tarantool-patches] Re: [PATCH 3/3] sql: too many autogenerated ids leads to SEGFAULT n.pettik
2018-11-22 18:09     ` Imeev Mergen
2018-11-27  8:50       ` n.pettik
2018-11-27 10:09 ` [tarantool-patches] Re: [PATCH 0/3] A set of small SQL fixes Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6128bdce00cc4c6d47c122897c195e30e0416613.1542635129.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=imeevma@gmail.com \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH 3/3] sql: too many autogenerated ids leads to SEGFAULT' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox