- moved lock handle functions to rsdir.h/cc

- created a scope guard to manage file lock handles
- added lock gards to PGP keyring read/writes.



git-svn-id: http://svn.code.sf.net/p/retroshare/code/branches/v0.5-OpenPGP@5216 b45a01b8-16f6-495d-af2f-9b41ad6348cc

libretroshare/src/pgp/pgphandler.cc

@@ -16,6 +16,7 @@ extern "C" {
 }
 #include "pgphandler.h"
 #include "retroshare/rsiface.h"		// For rsicontrol.
+#include "util/rsdir.h"		// For rsicontrol.
 
 PassphraseCallback PGPHandler::_passphrase_callback = NULL ;
 
@@ -67,9 +68,14 @@ void PGPHandler::setPassphraseCallback(PassphraseCallback cb)
 	_passphrase_callback = cb ;
 }
 
-PGPHandler::PGPHandler(const std::string& pubring, const std::string& secring)
-	: pgphandlerMtx(std::string("PGPHandler")), _pubring_path(pubring),_secring_path(secring)
+PGPHandler::PGPHandler(const std::string& pubring, const std::string& secring,const std::string& pgp_lock_filename)
+	: pgphandlerMtx(std::string("PGPHandler")), _pubring_path(pubring),_secring_path(secring),_pgp_lock_filename(pgp_lock_filename)
 {
+	_pubring_changed = false ;
+	_secring_changed = false ;
+
+	RsStackFileLock flck(_pgp_lock_filename) ;	// lock access to PGP directory.
+
 	if(_passphrase_callback == NULL)
 	{
 		std::cerr << "WARNING: before created a PGPHandler, you need to init the passphrase callback using PGPHandler::setPassphraseCallback()" << std::endl;
@@ -137,7 +143,6 @@ PGPHandler::PGPHandler(const std::string& pubring, const std::string& secring)
 	}
 
 	std::cerr << "Secring read successfully." << std::endl;
-
 }
 
 void PGPHandler::initCertificateInfo(PGPCertificateInfo& cert,const ops_keydata_t *keydata,uint32_t index)
@@ -338,6 +343,9 @@ bool PGPHandler::GeneratePGPCertificate(const std::string& name, const std::stri
 
 //	validateAndUpdateSignatures(_public_keyring_map[ pgpId.toStdString() ],getPublicKey(pgpId)) ;
 
+	_pubring_changed = true ;
+	_secring_changed = true ;
+
 	return true ;
 }
 
@@ -440,12 +448,25 @@ bool PGPHandler::LoadCertificateFromString(const std::string& pgp_cert,PGPIdType
 	ops_keyring_free(tmp_keyring) ;
 	free(tmp_keyring) ;
 
+	_pubring_changed = true ;
+
 	return true ;
 }
 
-bool PGPHandler::writePublicKeyring(const std::string& outfilename) const
+bool PGPHandler::writePublicKeyring() 
 {
-	return ops_write_keyring_to_file(_pubring,ops_false,outfilename.c_str()) ;
+	RsStackFileLock flck(_pgp_lock_filename) ; // locks access to pgp directory
+
+	_pubring_changed = false ;
+	return ops_write_keyring_to_file(_pubring,ops_false,_pubring_path.c_str()) ;
+}
+
+bool PGPHandler::writeSecretKeyring() 
+{
+	RsStackFileLock flck(_pgp_lock_filename) ; // locks access to pgp directory
+
+	_secring_changed = false ;
+	return ops_write_keyring_to_file(_secring,ops_false,_secring_path.c_str()) ;
 }
 
 bool PGPHandler::encryptTextToFile(const PGPIdType& key_id,const std::string& text,const std::string& outfile) 

libretroshare/src/pgp/pgphandler.h

@@ -52,7 +52,7 @@ class PGPCertificateInfo
 class PGPHandler
 {
 	public:
-		PGPHandler(const std::string& path_to_public_keyring, const std::string& path_to_secret_keyring) ;
+		PGPHandler(const std::string& path_to_public_keyring, const std::string& path_to_secret_keyring, const std::string& pgp_lock_file) ;
 
 		virtual ~PGPHandler() ;
 
@@ -79,10 +79,11 @@ class PGPHandler
 		void setAcceptConnexion(const PGPIdType&,bool) ;
 
 		// Write keyring
-		bool writePublicKeyring(const std::string& filename) const ;
+		bool publicKeyringChanged() const { return _pubring_changed ; }
+		bool secretKeyringChanged() const { return _secring_changed ; }
 
-		// Debug stuff.
-		virtual bool printKeys() const ;
+		bool writeSecretKeyring() ;
+		bool writePublicKeyring() ;
 
 		const PGPCertificateInfo *getCertificateInfo(const PGPIdType& id) const ;
 
@@ -92,6 +93,10 @@ class PGPHandler
 
 		static void setPassphraseCallback(PassphraseCallback cb) ;
 		static PassphraseCallback passphraseCallback() { return _passphrase_callback ; }
+
+		// Debug stuff.
+		virtual bool printKeys() const ;
+
 	private:
 		void initCertificateInfo(PGPCertificateInfo& cert,const ops_keydata_t *keydata,uint32_t i) ;
 		void validateAndUpdateSignatures(PGPCertificateInfo& cert,const ops_keydata_t *keydata) ;
@@ -111,6 +116,10 @@ class PGPHandler
 
 		const std::string _pubring_path ;
 		const std::string _secring_path ;
+		const std::string _pgp_lock_filename ;
+
+		bool _pubring_changed ;
+		bool _secring_changed ;
 
 		// Helper functions.
 		//

libretroshare/src/pqi/authgpg.cc

@@ -86,7 +86,7 @@ std::string pgp_pwd_callback(void * /*hook*/, const char *uid_hint, const char *
 	return password ;
 }
 
-void AuthGPG::init(const std::string& path_to_public_keyring,const std::string& path_to_secret_keyring)
+void AuthGPG::init(const std::string& path_to_public_keyring,const std::string& path_to_secret_keyring,const std::string& pgp_lock_file)
 {
 	if(_instance != NULL)
 	{
@@ -95,7 +95,7 @@ void AuthGPG::init(const std::string& path_to_public_keyring,const std::string&
 	}
 
 	PGPHandler::setPassphraseCallback(pgp_pwd_callback) ;
-	_instance = new AuthGPG(path_to_public_keyring,path_to_secret_keyring) ;
+	_instance = new AuthGPG(path_to_public_keyring,path_to_secret_keyring,pgp_lock_file) ;
 }
 
 void AuthGPG::exit()
@@ -108,9 +108,9 @@ void AuthGPG::exit()
 	}
 }
 
-AuthGPG::AuthGPG(const std::string& path_to_public_keyring,const std::string& path_to_secret_keyring)
+AuthGPG::AuthGPG(const std::string& path_to_public_keyring,const std::string& path_to_secret_keyring,const std::string& pgp_lock_file)
         :p3Config(CONFIG_TYPE_AUTHGPG), 
-		   PGPHandler(path_to_public_keyring,path_to_secret_keyring),
+		   PGPHandler(path_to_public_keyring,path_to_secret_keyring,pgp_lock_file),
 		   gpgMtxEngine("AuthGPG-engine"), 
 			gpgMtxData("AuthGPG-data"),
 			gpgKeySelected(false), 

libretroshare/src/pqi/authgpg.h

@@ -109,7 +109,7 @@ class AuthGPG: public p3Config, public RsThread, public PGPHandler
 {
 	public:
 
-		static void init(const std::string& path_to_pubring, const std::string& path_to_secring);
+		static void init(const std::string& path_to_pubring, const std::string& path_to_secring,const std::string& pgp_lock_file);
 		static void exit();
 		static AuthGPG *getAuthGPG() { return _instance ; }
 
@@ -220,7 +220,7 @@ class AuthGPG: public p3Config, public RsThread, public PGPHandler
 		virtual bool addService(AuthGPGService *service) ;
 
 	protected:
-		AuthGPG(const std::string& path_to_pubring, const std::string& path_to_secring);
+		AuthGPG(const std::string& path_to_pubring, const std::string& path_to_secring,const std::string& pgp_lock_file);
 		virtual ~AuthGPG();
 
 		/*****************************************************************/

libretroshare/src/rsserver/rsinit.cc

@@ -624,7 +624,7 @@ int RsInit::InitRetroShare(int argcIgnored, char **argvIgnored, bool strictCheck
 	if(!RsDirUtil::checkCreateDirectory(pgp_dir))
 		throw std::runtime_error("Cannot create pgp directory " + pgp_dir) ;
 
-	AuthGPG::init(pgp_dir + "/retroshare_public_keyring.gpg",pgp_dir + "/retroshare_secret_keyring.gpg");
+	AuthGPG::init(pgp_dir + "/retroshare_public_keyring.gpg",pgp_dir + "/retroshare_secret_keyring.gpg",pgp_dir + "/lock");
 
 	/* Initialize AuthGPG */
 	// if (AuthGPG::getAuthGPG()->InitAuth() == false) {
@@ -1183,75 +1183,9 @@ int      RsInit::GetPGPLoginDetails(const std::string& id, std::string &name, st
 int RsInit::LockConfigDirectory(const std::string& accountDir, std::string& lockFilePath)
 {
 	const std::string lockFile = accountDir + "/" + "lock";
-
 	lockFilePath = lockFile;
-/******************************** WINDOWS/UNIX SPECIFIC PART ******************/
-#ifndef WINDOWS_SYS
-	if(RsInitConfig::lockHandle != -1)
-		close(RsInitConfig::lockHandle);
 
-	// open the file in write mode, create it if necessary, truncate it (it should be empty)
-	RsInitConfig::lockHandle = open(lockFile.c_str(), O_WRONLY | O_CREAT | O_TRUNC,
-						S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-
-	if(RsInitConfig::lockHandle == -1)
-	{
-		std::cerr << "Could not open lock file " << lockFile.c_str() << std::flush;
-		perror(NULL);
-		return 2;
-	}
-
-	// see "man fcntl" for the details, in short: non blocking lock creation on the whole file contents
-	struct flock lockDetails;
-	lockDetails.l_type = F_WRLCK;
-	lockDetails.l_whence = SEEK_SET;
-	lockDetails.l_start = 0;
-	lockDetails.l_len = 0;
-
-	if(fcntl(RsInitConfig::lockHandle, F_SETLK, &lockDetails) == -1)
-	{
-		int fcntlErr = errno;
-		std::cerr << "Could not request lock on file " << lockFile.c_str() << std::flush;
-		perror(NULL);
-
-		// there's no lock so let's release the file handle immediately
-		close(RsInitConfig::lockHandle);
-		RsInitConfig::lockHandle = -1;
-
-		if(fcntlErr == EACCES || fcntlErr == EAGAIN)
-			return 1;
-		else
-			return 2;
-	}
-
-	return 0;
-#else
-	if (RsInitConfig::lockHandle) {
-		CloseHandle(RsInitConfig::lockHandle);
-	}
-
-	std::wstring wlockFile;
-	librs::util::ConvertUtf8ToUtf16(lockFile, wlockFile);
-
-	// open the file in write mode, create it if necessary
-	RsInitConfig::lockHandle = CreateFile(wlockFile.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL);
-
-	if (RsInitConfig::lockHandle == INVALID_HANDLE_VALUE) {
-		DWORD lasterror = GetLastError();
-
-		std::cerr << "Could not open lock file " << lockFile.c_str() << std::endl;
-		std::cerr << "Last error: " << lasterror << std::endl << std::flush;
-		perror(NULL);
-
-		if (lasterror == ERROR_SHARING_VIOLATION || lasterror == ERROR_ACCESS_DENIED) {
-			return 1;
-		}
-		return 2;
-	}
-
-	return 0;
-#endif
-/******************************** WINDOWS/UNIX SPECIFIC PART ******************/
+	return RsDirUtil::createLockFile(lockFile,RsInitConfig::lockHandle) ;
 }
 
 /*
@@ -1260,21 +1194,7 @@ int RsInit::LockConfigDirectory(const std::string& accountDir, std::string& lock
  */
 void	RsInit::UnlockConfigDirectory()
 {
-/******************************** WINDOWS/UNIX SPECIFIC PART ******************/
-#ifndef WINDOWS_SYS
-	if(RsInitConfig::lockHandle != -1)
-	{
-		close(RsInitConfig::lockHandle);
-		RsInitConfig::lockHandle = -1;
-	}
-#else
-	if(RsInitConfig::lockHandle)
-	{
-		CloseHandle(RsInitConfig::lockHandle);
-		RsInitConfig::lockHandle = NULL;
-	}
-#endif
-/******************************** WINDOWS/UNIX SPECIFIC PART ******************/
+	RsDirUtil::releaseLockFile(RsInitConfig::lockHandle) ;
 }
 
 

libretroshare/src/util/rsdir.cc

@@ -27,6 +27,7 @@
 // Includes for directory creation.
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/fcntl.h>
 #include <unistd.h>
 
 #include "util/rsdir.h"
@@ -851,6 +852,118 @@ std::string RsDirUtil::makePath(const std::string &path1, const std::string &pat
 	return path;
 }
 
+int RsDirUtil::createLockFile(const std::string& lock_file_path,int& lock_handle) 
+{
+	/******************************** WINDOWS/UNIX SPECIFIC PART ******************/
+#ifndef WINDOWS_SYS
+//	Suspended. The user should make sure he's not already using the file descriptor.
+//	if(lock_handle != -1)
+//		close(lock_handle);
+
+	// open the file in write mode, create it if necessary, truncate it (it should be empty)
+	lock_handle = open(lock_file_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	if(lock_handle == -1)
+	{
+		std::cerr << "Could not open lock file " << lock_file_path.c_str() << std::flush;
+		perror(NULL);
+		return 2;
+	}
+
+	// see "man fcntl" for the details, in short: non blocking lock creation on the whole file contents
+	struct flock lockDetails;
+	lockDetails.l_type = F_WRLCK;
+	lockDetails.l_whence = SEEK_SET;
+	lockDetails.l_start = 0;
+	lockDetails.l_len = 0;
+
+	if(fcntl(lock_handle, F_SETLK, &lockDetails) == -1)
+	{
+		int fcntlErr = errno;
+		std::cerr << "Could not request lock on file " << lock_file_path.c_str() << std::flush;
+		perror(NULL);
+
+		// there's no lock so let's release the file handle immediately
+		close(lock_handle);
+		lock_handle = -1;
+
+		if(fcntlErr == EACCES || fcntlErr == EAGAIN)
+			return 1;
+		else
+			return 2;
+	}
+
+	return 0;
+#else
+//	Suspended. The user should make sure he's not already using the file descriptor.
+//
+//	if (lock_handle) {
+//		CloseHandle(lock_handle);
+//	}
+
+	std::wstring wlockFile;
+	librs::util::ConvertUtf8ToUtf16(lock_file_path, wlockFile);
+
+	// open the file in write mode, create it if necessary
+	lock_handle = CreateFile(wlockFile.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL);
+
+	if (lock_handle == INVALID_HANDLE_VALUE) 
+	{
+		DWORD lasterror = GetLastError();
+
+		std::cerr << "Could not open lock file " << lock_file_path.c_str() << std::endl;
+		std::cerr << "Last error: " << lasterror << std::endl << std::flush;
+		perror(NULL);
+
+		if (lasterror == ERROR_SHARING_VIOLATION || lasterror == ERROR_ACCESS_DENIED) 
+			return 1;
+		
+		return 2;
+	}
+
+	return 0;
+#endif
+	/******************************** WINDOWS/UNIX SPECIFIC PART ******************/
+}
+
+void RsDirUtil::releaseLockFile(int lockHandle) 
+{
+	/******************************** WINDOWS/UNIX SPECIFIC PART ******************/
+#ifndef WINDOWS_SYS
+	if(lockHandle != -1)
+	{
+		close(lockHandle);
+		lockHandle = -1;
+	}
+#else
+	if(lockHandle)
+	{
+		CloseHandle(lockHandle);
+		lockHandle = 0;
+	}
+#endif
+	/******************************** WINDOWS/UNIX SPECIFIC PART ******************/
+}
+
+RsStackFileLock::RsStackFileLock(const std::string& file_path)
+{
+	while(RsDirUtil::createLockFile(file_path,_file_handle))
+	{
+		std::cerr << "Cannot acquire file lock " << file_path << ", waiting 1 sec." << std::endl;
+#ifdef WINDOWS_SYS
+		Sleep(1000) ;
+#else
+		sleep(1) ;
+#endif
+	}
+	std::cerr << "Acquired file handle " << _file_handle << ", lock file:" << file_path << std::endl;
+}
+RsStackFileLock::~RsStackFileLock()
+{
+	RsDirUtil::releaseLockFile(_file_handle) ;
+	std::cerr << "Released file lock with handle " << _file_handle << std::endl;
+}
+
 #if 0 // NOT ENABLED YET!
 /************************* WIDE STRING ***************************/
 /************************* WIDE STRING ***************************/

libretroshare/src/util/rsdir.h

@@ -36,6 +36,21 @@ class RsThread;
 
 #include <retroshare/rstypes.h>
 
+// This is a scope guard on a given file. Works like a mutex. Is blocking.
+// We could do that in another way: derive RsMutex into RsLockFileMutex, and
+// use RsStackMutex on it transparently. Only issue: this will cost little more 
+// because of the multiple inheritance.
+//
+class RsStackFileLock
+{
+	public:
+		RsStackFileLock(const std::string& file_path) ;
+		~RsStackFileLock() ;
+
+	private:
+		int _file_handle ;
+};
+
 namespace RsDirUtil {
 
 std::string 	getTopDir(const std::string&);
@@ -70,6 +85,15 @@ bool 		getFileHash(const std::string& filepath,std::string &hash, uint64_t &size
 
 Sha1CheckSum sha1sum(uint8_t *data,uint32_t size) ;
 
+// Creates a lock file with given path, and returns the lock handle
+// returns:
+// 	0: Success
+// 	1: Another instance already has the lock
+//    2 : Unexpected error
+int createLockFile(const std::string& lock_file_path,int& lock_handle) ;
+
+// Removes the lock file with specified handle.
+void releaseLockFile(int lockHandle) ;
 
 std::wstring 	getWideTopDir(std::wstring);
 std::wstring 	getWideRootDir(std::wstring);