View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001630 | GNUnet | datastore service | public | 2010-12-12 16:40 | 2010-12-23 23:50 |
| Reporter | LRN | Assigned To | LRN | ||
| Priority | normal | Severity | major | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Summary | 0001630: W32-specific fixes for datastore tests and a fix for sqlite plugin | ||||
| Description | 1) Same as datacache tests - argv[0] is used as-is for plugin name, resulting in a plugin_name "sqlite.exe", which prevents testcases from loading the right config file. The fix looks somewhat awkward, but it is also short. Personally, i think that it would have been better to use a single binary for all plugin tests, passing the plugin name as argv[1] (thus also eliminating the need for parsing argv[0]). 2) sqlite all-iterator prepares a statement, but iterator cleanup for this particular iterator does not finalize that statement (its counterpart, the basic iterator, works with iterator context, which includes two statements, and it does finalize them on cleanup, while all-iterator doesn't use context). I've fixed it to use NextContext as its iterator context, and to finalize its statement on cleanup. | ||||
| Steps To Reproduce | 1) Run datastore tests on W32 2) Run perf_plugin_datastore_sqlite on any platform. On non-W32 this bug is not apparent, because it manifests itself in sharing violation over the database file, which only occur on W32 because of its non-standard unlink() implementation, which cannot unlink files that are in use. The new logging lines in plugin unloading routine will expose this bug if some other statements get lost. | ||||
| Tags | No tags attached. | ||||
| Attached Files | 0001-Minor-W32-datastore-tweaks.-Fixed-stmt-finalizing-in.patch (10,991 bytes)
From e6e55b80de5c8ad85fdfec45709af907208f5d75 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= <lrn1986@gmail.com>
Date: Sun, 12 Dec 2010 11:37:21 +0300
Subject: [PATCH] Minor W32 datastore tweaks. Fixed stmt finalizing in all iterator in sqlite plugin
---
src/datastore/perf_datastore_api.c | 8 ++-
src/datastore/perf_plugin_datastore.c | 8 ++-
src/datastore/plugin_datastore_sqlite.c | 108 ++++++++++++++++++++-----
src/datastore/test_datastore_api.c | 8 ++-
src/datastore/test_datastore_api_management.c | 8 ++-
5 files changed, 116 insertions(+), 24 deletions(-)
diff --git a/src/datastore/perf_datastore_api.c b/src/datastore/perf_datastore_api.c
index aa0c932..7d402ce 100644
--- a/src/datastore/perf_datastore_api.c
+++ b/src/datastore/perf_datastore_api.c
@@ -407,13 +407,17 @@ int
main (int argc, char *argv[])
{
int ret;
- const char *pos;
+ char *pos;
char dir_name[128];
/* determine name of plugin to use */
plugin_name = argv[0];
while (NULL != (pos = strstr(plugin_name, "_")))
plugin_name = pos+1;
+ if (NULL != (pos = strstr(plugin_name, ".")))
+ pos[0] = 0;
+ else
+ pos = (char *) plugin_name;
GNUNET_snprintf (dir_name,
sizeof (dir_name),
@@ -428,6 +432,8 @@ main (int argc, char *argv[])
#endif
NULL);
ret = check ();
+ if (pos != plugin_name)
+ pos[0] = '.';
#if REPORT_ID
fprintf (stderr, "\n");
#endif
diff --git a/src/datastore/perf_plugin_datastore.c b/src/datastore/perf_plugin_datastore.c
index c55fb62..facf7be 100644
--- a/src/datastore/perf_plugin_datastore.c
+++ b/src/datastore/perf_plugin_datastore.c
@@ -415,13 +415,17 @@ int
main (int argc, char *argv[])
{
int ret;
- const char *pos;
+ char *pos;
char dir_name[128];
/* determine name of plugin to use */
plugin_name = argv[0];
while (NULL != (pos = strstr(plugin_name, "_")))
plugin_name = pos+1;
+ if (NULL != (pos = strstr(plugin_name, ".")))
+ pos[0] = 0;
+ else
+ pos = (char *) plugin_name;
GNUNET_snprintf (dir_name,
sizeof (dir_name),
@@ -436,6 +440,8 @@ main (int argc, char *argv[])
#endif
NULL);
ret = check ();
+ if (pos != plugin_name)
+ pos[0] = '.';
GNUNET_DISK_directory_remove (dir_name);
return ret;
diff --git a/src/datastore/plugin_datastore_sqlite.c b/src/datastore/plugin_datastore_sqlite.c
index 457f75c..1b83825 100644
--- a/src/datastore/plugin_datastore_sqlite.c
+++ b/src/datastore/plugin_datastore_sqlite.c
@@ -28,7 +28,7 @@
#include "gnunet_datastore_plugin.h"
#include <sqlite3.h>
-#define DEBUG_SQLITE GNUNET_YES
+#define DEBUG_SQLITE GNUNET_NO
/**
@@ -105,6 +105,11 @@ struct Plugin
sqlite3 *dbh;
/**
+ * Precompiled SQL for deletion.
+ */
+ sqlite3_stmt *delRow;
+
+ /**
* Precompiled SQL for update.
*/
sqlite3_stmt *updPrio;
@@ -145,9 +150,16 @@ sq_prepare (sqlite3 * dbh, const char *zSql,
sqlite3_stmt ** ppStmt)
{
char *dummy;
- return sqlite3_prepare_v2 (dbh,
- zSql,
- strlen (zSql), ppStmt, (const char **) &dummy);
+ int result;
+ result = sqlite3_prepare_v2 (dbh,
+ zSql,
+ strlen (zSql), ppStmt, (const char **) &dummy);
+#if DEBUG_SQLITE
+ GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
+ "sqlite",
+ "Prepared %p: %d\n", *ppStmt, result);
+#endif
+ return result;
}
@@ -324,12 +336,16 @@ database_setup (const struct GNUNET_CONFIGURATION_Handle *cfg,
"INSERT INTO gn080 (size, type, prio, "
"anonLevel, expire, hash, vhash, value) VALUES "
"(?, ?, ?, ?, ?, ?, ?, ?)",
- &plugin->insertContent) != SQLITE_OK))
+ &plugin->insertContent) != SQLITE_OK) ||
+ (sq_prepare (plugin->dbh,
+ "DELETE FROM gn080 WHERE _ROWID_ = ?",
+ &plugin->delRow) != SQLITE_OK))
{
LOG_SQLITE (plugin, NULL,
GNUNET_ERROR_TYPE_ERROR, "precompiling");
return GNUNET_SYSERR;
}
+
return GNUNET_OK;
}
@@ -342,11 +358,36 @@ database_setup (const struct GNUNET_CONFIGURATION_Handle *cfg,
static void
database_shutdown (struct Plugin *plugin)
{
+ int result;
+ if (plugin->delRow != NULL)
+ sqlite3_finalize (plugin->delRow);
if (plugin->updPrio != NULL)
sqlite3_finalize (plugin->updPrio);
if (plugin->insertContent != NULL)
sqlite3_finalize (plugin->insertContent);
- sqlite3_close (plugin->dbh);
+ result = sqlite3_close(plugin->dbh);
+ while (result == SQLITE_BUSY)
+ {
+ sqlite3_stmt *stmt;
+ GNUNET_log_from (GNUNET_ERROR_TYPE_WARNING,
+ "sqlite",
+ _("Tried to close sqlite without finalizing all prepared statements.\n"));
+ for (stmt = sqlite3_next_stmt(plugin->dbh, NULL); stmt != NULL; stmt = sqlite3_next_stmt(plugin->dbh, NULL))
+ {
+#if DEBUG_SQLITE
+ GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
+ "sqlite", "Closing statement %p\n", stmt);
+#endif
+ result = sqlite3_finalize(stmt);
+#if DEBUG_SQLITE
+ if (result != SQLITE_OK)
+ GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
+ "sqlite",
+ "Failed to close statement %p: %d\n", stmt, result);
+#endif
+ }
+ result = sqlite3_close(plugin->dbh);
+ }
GNUNET_free_non_null (plugin->fn);
}
@@ -362,26 +403,23 @@ static int
delete_by_rowid (struct Plugin* plugin,
unsigned long long rid)
{
- sqlite3_stmt *stmt;
- if (sq_prepare (plugin->dbh,
- "DELETE FROM gn080 WHERE _ROWID_ = ?", &stmt) != SQLITE_OK)
+ sqlite3_bind_int64 (plugin->delRow, 1, rid);
+ if (SQLITE_DONE != sqlite3_step (plugin->delRow))
{
LOG_SQLITE (plugin, NULL,
GNUNET_ERROR_TYPE_ERROR |
- GNUNET_ERROR_TYPE_BULK, "sq_prepare");
+ GNUNET_ERROR_TYPE_BULK, "sqlite3_step");
+ if (SQLITE_OK != sqlite3_reset (plugin->delRow))
+ LOG_SQLITE (plugin, NULL,
+ GNUNET_ERROR_TYPE_ERROR |
+ GNUNET_ERROR_TYPE_BULK, "sqlite3_reset");
return GNUNET_SYSERR;
}
- sqlite3_bind_int64 (stmt, 1, rid);
- if (SQLITE_DONE != sqlite3_step (stmt))
- {
+ if (SQLITE_OK != sqlite3_reset (plugin->delRow))
LOG_SQLITE (plugin, NULL,
GNUNET_ERROR_TYPE_ERROR |
- GNUNET_ERROR_TYPE_BULK, "sqlite3_step");
- sqlite3_finalize (stmt);
- return GNUNET_SYSERR;
- }
- sqlite3_finalize (stmt);
+ GNUNET_ERROR_TYPE_BULK, "sqlite3_reset");
return GNUNET_OK;
}
@@ -1167,7 +1205,7 @@ sqlite_plugin_iter_migration_order (void *cls,
* Call sqlite using the already prepared query to get
* the next result.
*
- * @param cls not used
+ * @param cls context with the prepared query
* @param nc context with the prepared query
* @return GNUNET_OK on success, GNUNET_SYSERR on error, GNUNET_NO if
* there are no more results
@@ -1185,6 +1223,10 @@ all_next_prepare (void *cls,
GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
"Asked to clean up iterator state.\n");
#endif
+ nc = (struct NextContext *)cls;
+ if (nc->stmt)
+ sqlite3_finalize (nc->stmt);
+ nc->stmt = NULL;
return GNUNET_SYSERR;
}
plugin = nc->plugin;
@@ -1241,7 +1283,7 @@ sqlite_plugin_iter_all_now (void *cls,
nc->iter_cls = iter_cls;
nc->stmt = stmt;
nc->prep = &all_next_prepare;
- nc->prep_cls = NULL;
+ nc->prep_cls = nc;
sqlite_next_request (nc, GNUNET_NO);
}
@@ -1595,10 +1637,26 @@ libgnunet_plugin_datastore_sqlite_done (void *cls)
struct GNUNET_DATASTORE_PluginFunctions *api = cls;
struct Plugin *plugin = api->cls;
+#if DEBUG_SQLITE
+ GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
+ "sqlite",
+ "sqlite plugin is doneing\n");
+#endif
+
if (plugin->next_task != GNUNET_SCHEDULER_NO_TASK)
{
+#if DEBUG_SQLITE
+ GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
+ "sqlite",
+ "Canceling next task\n");
+#endif
GNUNET_SCHEDULER_cancel (plugin->next_task);
plugin->next_task = GNUNET_SCHEDULER_NO_TASK;
+#if DEBUG_SQLITE
+ GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
+ "sqlite",
+ "Prep'ing next task\n");
+#endif
plugin->next_task_nc->prep (plugin->next_task_nc->prep_cls, NULL);
GNUNET_free (plugin->next_task_nc);
plugin->next_task_nc = NULL;
@@ -1606,6 +1664,11 @@ libgnunet_plugin_datastore_sqlite_done (void *cls)
fn = NULL;
if (plugin->drop_on_shutdown)
fn = GNUNET_strdup (plugin->fn);
+#if DEBUG_SQLITE
+ GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
+ "sqlite",
+ "Shutting down database\n");
+#endif
database_shutdown (plugin);
plugin->env = NULL;
GNUNET_free (api);
@@ -1617,6 +1680,11 @@ libgnunet_plugin_datastore_sqlite_done (void *cls)
fn);
GNUNET_free (fn);
}
+#if DEBUG_SQLITE
+ GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
+ "sqlite",
+ "sqlite plugin is finished doneing\n");
+#endif
return NULL;
}
diff --git a/src/datastore/test_datastore_api.c b/src/datastore/test_datastore_api.c
index de04655..94d068a 100644
--- a/src/datastore/test_datastore_api.c
+++ b/src/datastore/test_datastore_api.c
@@ -678,13 +678,17 @@ int
main (int argc, char *argv[])
{
int ret;
- const char *pos;
+ char *pos;
char dir_name[128];
/* determine name of plugin to use */
plugin_name = argv[0];
while (NULL != (pos = strstr(plugin_name, "_")))
plugin_name = pos+1;
+ if (NULL != (pos = strstr(plugin_name, ".")))
+ pos[0] = 0;
+ else
+ pos = (char *) plugin_name;
GNUNET_snprintf (dir_name,
sizeof (dir_name),
@@ -699,6 +703,8 @@ main (int argc, char *argv[])
#endif
NULL);
ret = check ();
+ if (pos != plugin_name)
+ pos[0] = '.';
GNUNET_DISK_directory_remove (dir_name);
return ret;
}
diff --git a/src/datastore/test_datastore_api_management.c b/src/datastore/test_datastore_api_management.c
index 03e14d6..5778f0b 100644
--- a/src/datastore/test_datastore_api_management.c
+++ b/src/datastore/test_datastore_api_management.c
@@ -388,13 +388,17 @@ main (int argc, char *argv[])
{
int ret;
- const char *pos;
+ char *pos;
char dir_name[128];
/* determine name of plugin to use */
plugin_name = argv[0];
while (NULL != (pos = strstr(plugin_name, "_")))
plugin_name = pos+1;
+ if (NULL != (pos = strstr(plugin_name, ".")))
+ pos[0] = 0;
+ else
+ pos = (char *) plugin_name;
GNUNET_snprintf (dir_name,
sizeof (dir_name),
@@ -409,6 +413,8 @@ main (int argc, char *argv[])
#endif
NULL);
ret = check ();
+ if (pos != plugin_name)
+ pos[0] = '.';
GNUNET_DISK_directory_remove (dir_name);
return ret;
}
--
1.7.3.1.msysgit.0
| ||||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2010-12-12 16:40 | LRN | New Issue | |
| 2010-12-12 16:40 | LRN | File Added: 0001-Minor-W32-datastore-tweaks.-Fixed-stmt-finalizing-in.patch | |
| 2010-12-19 19:23 | Christian Grothoff | Note Added: 0004207 | |
| 2010-12-19 19:23 | Christian Grothoff | Status | new => resolved |
| 2010-12-19 19:23 | Christian Grothoff | Resolution | open => fixed |
| 2010-12-19 19:23 | Christian Grothoff | Assigned To | => Christian Grothoff |
| 2010-12-19 19:23 | Christian Grothoff | Assigned To | Christian Grothoff => LRN |
| 2010-12-23 23:50 | Christian Grothoff | Status | resolved => closed |
