From 81bbef26099eb54f4dcc32e45b5e0416857a330d Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Sun, 23 Sep 2012 12:55:05 +0200 Subject: [PATCH] add LOCK() for proxy related data-structures - fix #1560 by properly locking proxy related data-structures - update GetProxy() and introduce GetNameProxy() to be able to use a thread-safe local copy from proxyInfo and nameproxyInfo - update usage of GetProxy() all over the source to match the new behaviour, as it now fills a full proxyType object - rename GetNameProxy() into HaveNameProxy() to be more clear --- src/net.cpp | 4 +-- src/netbase.cpp | 41 +++++++++++++++++------- src/netbase.h | 6 ++-- src/qt/optionsmodel.cpp | 69 +++++++++++++++++++++++------------------ src/rpcwallet.cpp | 6 ++-- 5 files changed, 77 insertions(+), 49 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 7c327f5d3..5b5510a5b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1185,7 +1185,7 @@ void ThreadDNSAddressSeed2(void* parg) printf("Loading addresses from DNS seeds (could take a while)\n"); for (unsigned int seed_idx = 0; seed_idx < ARRAYLEN(strDNSSeed); seed_idx++) { - if (GetNameProxy()) { + if (HaveNameProxy()) { AddOneShot(strDNSSeed[seed_idx][1]); } else { vector vaddr; @@ -1529,7 +1529,7 @@ void ThreadOpenAddedConnections2(void* parg) if (mapArgs.count("-addnode") == 0) return; - if (GetNameProxy()) { + if (HaveNameProxy()) { while(!fShutdown) { BOOST_FOREACH(string& strAddNode, mapMultiArgs["-addnode"]) { CAddress addr; diff --git a/src/netbase.cpp b/src/netbase.cpp index daa8a8d07..95d64939b 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -5,6 +5,7 @@ #include "netbase.h" #include "util.h" +#include "sync.h" #ifndef WIN32 #include @@ -16,9 +17,9 @@ using namespace std; // Settings -typedef std::pair proxyType; static proxyType proxyInfo[NET_MAX]; static proxyType nameproxyInfo; +static CCriticalSection cs_proxyInfos; int nConnectTimeout = 5000; bool fNameLookup = false; @@ -432,15 +433,17 @@ bool SetProxy(enum Network net, CService addrProxy, int nSocksVersion) { return false; if (nSocksVersion != 0 && !addrProxy.IsValid()) return false; + LOCK(cs_proxyInfos); proxyInfo[net] = std::make_pair(addrProxy, nSocksVersion); return true; } -bool GetProxy(enum Network net, CService &addrProxy) { +bool GetProxy(enum Network net, proxyType &proxyInfoOut) { assert(net >= 0 && net < NET_MAX); + LOCK(cs_proxyInfos); if (!proxyInfo[net].second) return false; - addrProxy = proxyInfo[net].first; + proxyInfoOut = proxyInfo[net]; return true; } @@ -449,16 +452,27 @@ bool SetNameProxy(CService addrProxy, int nSocksVersion) { return false; if (nSocksVersion != 0 && !addrProxy.IsValid()) return false; + LOCK(cs_proxyInfos); nameproxyInfo = std::make_pair(addrProxy, nSocksVersion); return true; } -bool GetNameProxy() { +bool GetNameProxy(proxyType &nameproxyInfoOut) { + LOCK(cs_proxyInfos); + if (!nameproxyInfo.second) + return false; + nameproxyInfoOut = nameproxyInfo; + return true; +} + +bool HaveNameProxy() { + LOCK(cs_proxyInfos); return nameproxyInfo.second != 0; } bool IsProxy(const CNetAddr &addr) { - for (int i=0; i proxyType; + enum Network ParseNetwork(std::string net); void SplitHostPort(std::string in, int &portOut, std::string &hostOut); bool SetProxy(enum Network net, CService addrProxy, int nSocksVersion = 5); -bool GetProxy(enum Network net, CService &addrProxy); +bool GetProxy(enum Network net, proxyType &proxyInfoOut); bool IsProxy(const CNetAddr &addr); bool SetNameProxy(CService addrProxy, int nSocksVersion = 5); -bool GetNameProxy(); +bool HaveNameProxy(); bool LookupHost(const char *pszName, std::vector& vIP, unsigned int nMaxSolutions = 0, bool fAllowLookup = true); bool LookupHostNumeric(const char *pszName, std::vector& vIP, unsigned int nMaxSolutions = 0); bool Lookup(const char *pszName, CService& addr, int portDefault = 0, bool fAllowLookup = true); diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index caa33414b..756fe61ec 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -145,18 +145,18 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const case ProxyUse: return settings.value("fUseProxy", false); case ProxyIP: { - CService addrProxy; - if (GetProxy(NET_IPV4, addrProxy)) - return QVariant(QString::fromStdString(addrProxy.ToStringIP())); + proxyType proxy; + if (GetProxy(NET_IPV4, proxy)) + return QVariant(QString::fromStdString(proxy.first.ToStringIP())); else return QVariant(QString::fromStdString("127.0.0.1")); } case ProxyPort: { - CService addrProxy; - if (GetProxy(NET_IPV4, addrProxy)) - return QVariant(addrProxy.GetPort()); + proxyType proxy; + if (GetProxy(NET_IPV4, proxy)) + return QVariant(proxy.first.GetPort()); else - return 9050; + return QVariant(9050); } case ProxySocksVersion: return settings.value("nSocksVersion", 5); @@ -176,6 +176,7 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const } return QVariant(); } + bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, int role) { bool successful = true; /* set to false on parse error */ @@ -204,29 +205,37 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in settings.setValue("fUseProxy", value.toBool()); ApplyProxySettings(); break; - case ProxyIP: - { - CService addrProxy("127.0.0.1", 9050); - GetProxy(NET_IPV4, addrProxy); - CNetAddr addr(value.toString().toStdString()); - addrProxy.SetIP(addr); - settings.setValue("addrProxy", addrProxy.ToStringIPPort().c_str()); - successful = ApplyProxySettings(); - } - break; - case ProxyPort: - { - CService addrProxy("127.0.0.1", 9050); - GetProxy(NET_IPV4, addrProxy); - addrProxy.SetPort(value.toInt()); - settings.setValue("addrProxy", addrProxy.ToStringIPPort().c_str()); - successful = ApplyProxySettings(); - } - break; - case ProxySocksVersion: - settings.setValue("nSocksVersion", value.toInt()); - ApplyProxySettings(); - break; + case ProxyIP: { + proxyType proxy; + proxy.first = CService("127.0.0.1", 9050); + GetProxy(NET_IPV4, proxy); + + CNetAddr addr(value.toString().toStdString()); + proxy.first.SetIP(addr); + settings.setValue("addrProxy", proxy.first.ToStringIPPort().c_str()); + successful = ApplyProxySettings(); + } + break; + case ProxyPort: { + proxyType proxy; + proxy.first = CService("127.0.0.1", 9050); + GetProxy(NET_IPV4, proxy); + + proxy.first.SetPort(value.toInt()); + settings.setValue("addrProxy", proxy.first.ToStringIPPort().c_str()); + successful = ApplyProxySettings(); + } + break; + case ProxySocksVersion: { + proxyType proxy; + proxy.second = 5; + GetProxy(NET_IPV4, proxy); + + proxy.second = value.toInt(); + settings.setValue("nSocksVersion", proxy.second); + successful = ApplyProxySettings(); + } + break; case Fee: nTransactionFee = value.toLongLong(); settings.setValue("nTransactionFee", nTransactionFee); diff --git a/src/rpcwallet.cpp b/src/rpcwallet.cpp index 66670d093..fda921a64 100644 --- a/src/rpcwallet.cpp +++ b/src/rpcwallet.cpp @@ -62,8 +62,8 @@ Value getinfo(const Array& params, bool fHelp) "getinfo\n" "Returns an object containing various state info."); - CService addrProxy; - GetProxy(NET_IPV4, addrProxy); + proxyType proxy; + GetProxy(NET_IPV4, proxy); Object obj; obj.push_back(Pair("version", (int)CLIENT_VERSION)); @@ -72,7 +72,7 @@ Value getinfo(const Array& params, bool fHelp) obj.push_back(Pair("balance", ValueFromAmount(pwalletMain->GetBalance()))); obj.push_back(Pair("blocks", (int)nBestHeight)); obj.push_back(Pair("connections", (int)vNodes.size())); - obj.push_back(Pair("proxy", (addrProxy.IsValid() ? addrProxy.ToStringIPPort() : string()))); + obj.push_back(Pair("proxy", (proxy.first.IsValid() ? proxy.first.ToStringIPPort() : string()))); obj.push_back(Pair("difficulty", (double)GetDifficulty())); obj.push_back(Pair("testnet", fTestNet)); obj.push_back(Pair("keypoololdest", (boost::int64_t)pwalletMain->GetOldestKeyPoolTime()));