gnucash maint: Multiple changes pushed
John Ralls
jralls at code.gnucash.org
Mon Sep 6 18:23:31 EDT 2021
Updated via https://github.com/Gnucash/gnucash/commit/901fea15 (commit)
via https://github.com/Gnucash/gnucash/commit/38cd06e5 (commit)
via https://github.com/Gnucash/gnucash/commit/06652082 (commit)
via https://github.com/Gnucash/gnucash/commit/e4619fda (commit)
from https://github.com/Gnucash/gnucash/commit/079a9003 (commit)
commit 901fea158f29f088e2cd9d01041b842b80af1b6f
Merge: 079a90030 38cd06e54
Author: John Ralls <jralls at ceridwen.us>
Date: Mon Sep 6 15:22:49 2021 -0700
Merge Simon Arlott's 'xml-locking-fixes' into maint.
commit 38cd06e54a016f6d039ff97ecffaf4c0f8eb7191
Author: John Ralls <jralls at ceridwen.us>
Date: Mon Sep 6 15:18:34 2021 -0700
Remove the lock-file-link-count test from the XML backend.
This was an effort of somewhat dubious value to detect if a process
on another ocmputer had a hsrd-link to a lockfile on an NFS share.
NFS is rarely used now and SMB doesn't support hard links so this check
adds complexity with no real value.
diff --git a/libgnucash/backend/xml/gnc-xml-backend.cpp b/libgnucash/backend/xml/gnc-xml-backend.cpp
index f6cb55a78..8b8a3059f 100644
--- a/libgnucash/backend/xml/gnc-xml-backend.cpp
+++ b/libgnucash/backend/xml/gnc-xml-backend.cpp
@@ -669,90 +669,7 @@ GncXmlBackend::get_file_lock ()
return false;
}
- /* OK, now work around some NFS atomic lock race condition
- * mumbo-jumbo. We do this by linking a unique file, and
- * then examining the link count. At least that's what the
- * NFS programmers guide suggests.
- * Note: the "unique filename" must be unique for the
- * triplet filename-host-process, otherwise accidental
- * aliases can occur.
- */
-
- /* apparently, even this code may not work for some NFS
- * implementations. In the long run, I am told that
- * ftp.debian.org
- * /pub/debian/dists/unstable/main/source/libs/liblockfile_0.1-6.tar.gz
- * provides a better long-term solution.
- */
-
-#ifndef G_OS_WIN32
- auto path = m_lockfile.find_last_of('.');
- std::stringstream linkfile;
- if (path != std::string::npos)
- linkfile << m_lockfile.substr(0, path);
- else
- linkfile << m_lockfile;
- linkfile << "." << gethostid() << "." << getpid() << ".LNK";
- rc = link (m_lockfile.c_str(), linkfile.str().c_str());
- if (rc)
- {
- /* If hard links aren't supported, just allow the lock. */
- if (errno == EPERM || errno == ENOSYS
-# ifdef EOPNOTSUPP
- || errno == EOPNOTSUPP
-# endif
-# ifdef ENOTSUP
- || errno == ENOTSUP
-# endif
- )
- {
- return true;
- }
-
- /* Otherwise, something else is wrong. */
- set_error(ERR_BACKEND_LOCKED);
- g_unlink (linkfile.str().c_str());
- close (m_lockfd);
- m_lockfd = -1;
- g_unlink (m_lockfile.c_str());
- m_lockfile.clear();
- return false;
- }
-
- rc = g_stat (m_lockfile.c_str(), &statbuf);
- if (rc)
- {
- /* oops .. stat failed! This can't happen! */
- set_error(ERR_BACKEND_LOCKED);
- std::string msg{"Failed to stat lockfile "};
- set_message(msg + m_lockfile);
- g_unlink (linkfile.str().c_str());
- close (m_lockfd);
- m_lockfd = -1;
- g_unlink (m_lockfile.c_str());
- m_lockfile.clear();
- return false;
- }
-
- if (statbuf.st_nlink != 2)
- {
- set_error(ERR_BACKEND_LOCKED);
- g_unlink (linkfile.str().c_str());
- close (m_lockfd);
- m_lockfd = -1;
- g_unlink (m_lockfile.c_str());
- m_lockfile.clear();
- return false;
- }
-
- m_linkfile = linkfile.str();
- return true;
-
-#else /* ifndef G_OS_WIN32 */
- /* On windows, there is no NFS and the open(,O_CREAT | O_EXCL)
- is sufficient for locking. */
return true;
-#endif /* ifndef G_OS_WIN32 */
}
bool
commit 066520829970fd14c45f953884e6aeeb7d4e1202
Author: Simon Arlott <sa.me.uk>
Date: Tue Aug 24 09:01:16 2021 +0100
xml-backend: Lock file is deleted even if the lock is not acquired
The lock file is set in m_lockfile and then unlinked in session_end even if
the lock was not acquired.
Clear m_lockfile if locking was not successful.
diff --git a/libgnucash/backend/xml/gnc-xml-backend.cpp b/libgnucash/backend/xml/gnc-xml-backend.cpp
index d9e3c9d3d..f6cb55a78 100644
--- a/libgnucash/backend/xml/gnc-xml-backend.cpp
+++ b/libgnucash/backend/xml/gnc-xml-backend.cpp
@@ -641,6 +641,7 @@ GncXmlBackend::get_file_lock ()
{
/* oops .. file is locked by another user .. */
set_error(ERR_BACKEND_LOCKED);
+ m_lockfile.clear();
return false;
}
@@ -664,6 +665,7 @@ GncXmlBackend::get_file_lock ()
PWARN ("Unable to create the lockfile %s: %s",
m_lockfile.c_str(), strerror(errno));
set_error(be_err);
+ m_lockfile.clear();
return false;
}
@@ -713,6 +715,7 @@ GncXmlBackend::get_file_lock ()
close (m_lockfd);
m_lockfd = -1;
g_unlink (m_lockfile.c_str());
+ m_lockfile.clear();
return false;
}
@@ -727,6 +730,7 @@ GncXmlBackend::get_file_lock ()
close (m_lockfd);
m_lockfd = -1;
g_unlink (m_lockfile.c_str());
+ m_lockfile.clear();
return false;
}
@@ -737,6 +741,7 @@ GncXmlBackend::get_file_lock ()
close (m_lockfd);
m_lockfd = -1;
g_unlink (m_lockfile.c_str());
+ m_lockfile.clear();
return false;
}
commit e4619fdae6eeb481faf2385a94a7ba8e21183a0d
Author: Simon Arlott <sa.me.uk>
Date: Tue Aug 24 08:54:06 2021 +0100
xml-backend: Don't try to close m_lockfd if it's not open
m_lockfd is not initialised. If the file is locked then it will not be set
before session_end and close() will be called on an uninitialised int.
Initialise it to -1 in the class definition.
Consistently use -1 instead of "< 0" or "< 1" as the definition of invalid.
Always set it to -1 after closing it.
diff --git a/libgnucash/backend/xml/gnc-xml-backend.cpp b/libgnucash/backend/xml/gnc-xml-backend.cpp
index 236afa80b..d9e3c9d3d 100644
--- a/libgnucash/backend/xml/gnc-xml-backend.cpp
+++ b/libgnucash/backend/xml/gnc-xml-backend.cpp
@@ -171,8 +171,11 @@ GncXmlBackend::session_end()
if (!m_linkfile.empty())
g_unlink (m_linkfile.c_str());
- if (m_lockfd > 0)
+ if (m_lockfd != -1)
+ {
close (m_lockfd);
+ m_lockfd = -1;
+ }
if (!m_lockfile.empty())
{
@@ -643,7 +646,7 @@ GncXmlBackend::get_file_lock ()
m_lockfd = g_open (m_lockfile.c_str(), O_RDWR | O_CREAT | O_EXCL ,
S_IRUSR | S_IWUSR);
- if (m_lockfd < 0)
+ if (m_lockfd == -1)
{
/* oops .. we can't create the lockfile .. */
switch (errno)
@@ -708,6 +711,7 @@ GncXmlBackend::get_file_lock ()
set_error(ERR_BACKEND_LOCKED);
g_unlink (linkfile.str().c_str());
close (m_lockfd);
+ m_lockfd = -1;
g_unlink (m_lockfile.c_str());
return false;
}
@@ -721,6 +725,7 @@ GncXmlBackend::get_file_lock ()
set_message(msg + m_lockfile);
g_unlink (linkfile.str().c_str());
close (m_lockfd);
+ m_lockfd = -1;
g_unlink (m_lockfile.c_str());
return false;
}
@@ -730,6 +735,7 @@ GncXmlBackend::get_file_lock ()
set_error(ERR_BACKEND_LOCKED);
g_unlink (linkfile.str().c_str());
close (m_lockfd);
+ m_lockfd = -1;
g_unlink (m_lockfile.c_str());
return false;
}
diff --git a/libgnucash/backend/xml/gnc-xml-backend.hpp b/libgnucash/backend/xml/gnc-xml-backend.hpp
index 1e6ff76e8..ab98a7b61 100644
--- a/libgnucash/backend/xml/gnc-xml-backend.hpp
+++ b/libgnucash/backend/xml/gnc-xml-backend.hpp
@@ -60,7 +60,7 @@ private:
std::string m_dirname;
std::string m_lockfile;
std::string m_linkfile;
- int m_lockfd;
+ int m_lockfd = -1;
QofBook* m_book = nullptr; /* The primary, main open book */
};
Summary of changes:
libgnucash/backend/xml/gnc-xml-backend.cpp | 86 +++---------------------------
libgnucash/backend/xml/gnc-xml-backend.hpp | 2 +-
2 files changed, 8 insertions(+), 80 deletions(-)
More information about the gnucash-changes
mailing list