wrong server saved in queue.sqlite3

Need help with FileZilla Client? Something does not work as expected? In this forum you may find an answer.

Moderator: Project members

Post Reply
Message
Author
twu2
425 Can't open data connection
Posts: 45
Joined: 2005-02-26 16:54

wrong server saved in queue.sqlite3

#1 Post by twu2 » 2011-10-25 02:40

if more than one server in queue and quit filezilla client, all files in queue will be saved as 1st server only.

Code: Select all

--- FileZilla3/src/interface/queue_storage.cpp.orig	2011-10-25 10:25:51 +0800
+++ FileZilla3/src/interface/queue_storage.cpp	2011-10-25 12:54:53 +0800
@@ -694,7 +694,8 @@
 	if (ret)
 	{
 		sqlite3_int64 serverId = sqlite3_last_insert_rowid(db_);
-        Bind(insertFileQuery_, file_table_column_names::server, serverId);
+        // insertFileQuery_ will be reset in SaveFile(), we assign this value after reset it
+        //Bind(insertFileQuery_, file_table_column_names::server, serverId);
 
 		const std::vector<CQueueItem*>& children = item.GetChildren();
 		for (std::vector<CQueueItem*>::const_iterator it = children.begin() + item.GetRemovedAtFront(); it != children.end(); ++it)
@@ -717,6 +718,8 @@
 
 	sqlite3_reset(insertFileQuery_);
 
+    // assign server id here
+    Bind(insertFileQuery_, file_table_column_names::server, server.GetValue());
 	Bind(insertFileQuery_, file_table_column_names::source_file, file.GetSourceFile());
 	if (file.GetTargetFile().empty())
 		BindNull(insertFileQuery_, file_table_column_names::target_file);
@@ -761,6 +764,8 @@
 {
 	sqlite3_reset(insertFileQuery_);
 
+    // assign server id here
+    Bind(insertFileQuery_, file_table_column_names::server, server.GetValue());
 	if (download)
 		BindNull(insertFileQuery_, file_table_column_names::source_file);
 	else

updated the patch for folder item also.

User avatar
botg
Site Admin
Posts: 33182
Joined: 2004-02-23 20:49
First name: Tim
Last name: Kosse
Contact:

Re: wrong server saved in queue.sqlite3

#2 Post by botg » 2011-10-25 06:29

The bound server value does not get overwritten or otherwise changed in either SaveFile or SaveDirectory and thus is the same for files belonging to the same server. Your patch is semantically equivalent but only introduces unnecessary binding of the same value over and over again.

From http://www.sqlite.org/c3ref/reset.html:
Any SQL statement variables that had values bound to them using the sqlite3_bind_*() API retain their values
The sqlite3_reset(S) interface does not change the values of any bindings on the prepared statement S.

twu2
425 Can't open data connection
Posts: 45
Joined: 2005-02-26 16:54

Re: wrong server saved in queue.sqlite3

#3 Post by twu2 » 2011-10-25 07:13

yes, the sqlite3 documentation said it won't affect.

but there's bug in filezilla 3.5.1 now, just connect to server A, add file A1 to queue, then connect to server B, add B1 to queue, then quit filezilla.
the data in queue.sqlite3 will point B1 and A1 to server A.
run filezilla again, you can see all files is under server A in queue, it's not accurate.

after apply the patch, no above issue anymore.

User avatar
boco
Contributor
Posts: 25286
Joined: 2006-05-01 03:28
Location: Germany

Re: wrong server saved in queue.sqlite3

#4 Post by boco » 2011-10-25 11:07

Bug confirmed. All files get assigned to the first server in the queue after a restart.
### BEGIN SIGNATURE BLOCK ###
No support requests per PM! You will NOT get any reply!!!
FTP connection problems? Do yourself a favor and read Network Configuration.
FileZilla connection test: https://filezilla-project.org/conntest.php
### END SIGNATURE BLOCK ###

User avatar
botg
Site Admin
Posts: 33182
Joined: 2004-02-23 20:49
First name: Tim
Last name: Kosse
Contact:

Re: wrong server saved in queue.sqlite3

#5 Post by botg » 2011-10-25 16:29

The problem must be something else, I fail to see how this patch can affect it.

User avatar
botg
Site Admin
Posts: 33182
Joined: 2004-02-23 20:49
First name: Tim
Last name: Kosse
Contact:

Re: wrong server saved in queue.sqlite3

#6 Post by botg » 2011-10-25 17:22

A fix has been committed to the repository. When testing your own patch, you should have notices that for each additional server, the first file would still have been added to the previous server entry.

twu2
425 Can't open data connection
Posts: 45
Joined: 2005-02-26 16:54

Re: wrong server saved in queue.sqlite3

#7 Post by twu2 » 2011-10-26 08:31

ok. it was fixed now.

based on the sqlite3_step() document:

Code: Select all

For all versions of SQLite up to and including 3.6.23.1, a call to sqlite3_reset() was required after sqlite3_step() returned anything other than SQLITE_ROW before any subsequent invocation of sqlite3_step(). Failure to reset the prepared statement using sqlite3_reset() would result in an SQLITE_MISUSE return from sqlite3_step(). But after version 3.6.23.1, sqlite3_step() began calling sqlite3_reset() automatically in this circumstance rather than returning SQLITE_MISUSE. This is not considered a compatibility break because any application that ever receives an SQLITE_MISUSE error is broken by definition. The SQLITE_OMIT_AUTORESET compile-time option can be used to restore the legacy behavior.
it's better to call sqlite3_reset() after sqlite3_step(). so do we need to change the flow for other sql call in queue_storage.cpp? (no issue for the result now, but in the source code, it call sqlite_reset() before sqlite3_step(), not after...

User avatar
botg
Site Admin
Posts: 33182
Joined: 2004-02-23 20:49
First name: Tim
Last name: Kosse
Contact:

Re: wrong server saved in queue.sqlite3

#8 Post by botg » 2011-10-26 20:44

Might be a good idea to change it, though I don't think it's really needed. During my code review I didn't spot another location where the order is problematic as was the case with the server id.

If you can provide a patch though, I'd happily apply it though if it doesn't cause any problems.

twu2
425 Can't open data connection
Posts: 45
Joined: 2005-02-26 16:54

Re: wrong server saved in queue.sqlite3

#9 Post by twu2 » 2011-10-27 09:18

here is the patch for move sqlite3_reset() after sqlite3_step():

Code: Select all

--- FileZilla3/src/interface/queue_storage.orig.cpp	2011-10-26 10:49:31 +0800
+++ FileZilla3/src/interface/queue_storage.cpp	2011-10-27 17:11:24 +0800
@@ -223,8 +223,6 @@
 	if (!selectLocalPathQuery_)
 		return;
 
-	sqlite3_reset(selectLocalPathQuery_);
-
 	int res;
 	do
 	{
@@ -237,6 +235,8 @@
 			if (id > 0 && !localPathRaw.empty() && localPath.SetPath(localPathRaw))
 				reverseLocalPaths_[id] = localPath;
 		}
+        else
+            sqlite3_reset(selectLocalPathQuery_);
 	}
 	while (res == SQLITE_BUSY || res == SQLITE_ROW);
 }
@@ -247,8 +247,6 @@
 	if (!selectRemotePathQuery_)
 		return;
 
-	sqlite3_reset(selectRemotePathQuery_);
-
 	int res;
 	do
 	{
@@ -261,6 +259,8 @@
 			if (id > 0 && !remotePathRaw.empty() && remotePath.SetSafePath(remotePathRaw))
 				reverseRemotePaths_[id] = remotePath;
 		}
+        else
+            sqlite3_reset(selectRemotePathQuery_);
 	}
 	while (res == SQLITE_BUSY || res == SQLITE_ROW);
 }
@@ -328,7 +328,6 @@
 	if (it != localPaths_.end())
 		return it->second;
 
-	sqlite3_reset(insertLocalPathQuery_);
 	Bind(insertLocalPathQuery_, path_table_column_names::path, path.GetPath());
 
 	int res;
@@ -336,6 +335,8 @@
 		res = sqlite3_step(insertLocalPathQuery_);
 	} while (res == SQLITE_BUSY);
 
+    sqlite3_reset(insertLocalPathQuery_);
+
 	if (res == SQLITE_DONE)
 	{
 		wxLongLong_t id = sqlite3_last_insert_rowid(db_);
@@ -354,7 +355,6 @@
 	if (it != remotePaths_.end())
 		return it->second;
 
-	sqlite3_reset(insertRemotePathQuery_);
 	Bind(insertRemotePathQuery_, path_table_column_names::path, path.GetSafePath());
 
 	int res;
@@ -362,6 +362,8 @@
 		res = sqlite3_step(insertRemotePathQuery_);
 	} while (res == SQLITE_BUSY);
 
+    sqlite3_reset(insertRemotePathQuery_);
+
 	if (res == SQLITE_DONE)
 	{
 		wxLongLong_t id = sqlite3_last_insert_rowid(db_);
@@ -691,6 +693,8 @@
 		res = sqlite3_step(insertServerQuery_);
 	} while (res == SQLITE_BUSY);
 
+    sqlite3_reset(insertServerQuery_);
+
 	bool ret = res == SQLITE_DONE;
 	if (ret)
 	{
@@ -1070,7 +1074,6 @@
 		{
 			d_->ReadLocalPaths();
 			d_->ReadRemotePaths();
-			sqlite3_reset(d_->selectServersQuery_);
 		}
 
 		while (true)
@@ -1088,10 +1091,14 @@
 				if (ret > 0)
 					break;
 			}
-			else if (res == SQLITE_DONE)
-			{
-				ret = 0;
-				break;
+			else
+            {
+                sqlite3_reset(d_->selectServersQuery_);
+                if (res == SQLITE_DONE)
+                {
+                    ret = 0;
+                    break;
+                }
 			}
 		}
 	}
@@ -1112,7 +1119,6 @@
 	{
 		if (server > 0)
 		{
-			sqlite3_reset(d_->selectFilesQuery_);
 			sqlite3_bind_int64(d_->selectFilesQuery_, 1, server);
 		}
 
@@ -1131,10 +1137,14 @@
 				if (ret > 0)
 					break;
 			}
-			else if (res == SQLITE_DONE)
-			{
-				ret = 0;
-				break;
+			else
+            {
+                sqlite3_reset(d_->selectFilesQuery_);
+                if (res == SQLITE_DONE)
+                {
+                    ret = 0;
+                    break;
+                }
 			}
 		}
 	}

User avatar
botg
Site Admin
Posts: 33182
Joined: 2004-02-23 20:49
First name: Tim
Last name: Kosse
Contact:

Re: wrong server saved in queue.sqlite3

#10 Post by botg » 2011-11-01 14:53

Applied with a few small modifications to keep the external semantics of the storage class the same. Also fixed a problem with SQLITE_BUSY not being handled correctly by your patch.

Post Reply