linuxdcpp-team team mailing list archive
-
linuxdcpp-team team
-
Mailing list archive
-
Message #05756
[Branch ~dcplusplus-team/dcplusplus/trunk] Rev 2961: Avoid deadlocks when changing user matchings
------------------------------------------------------------
revno: 2961
committer: poy <poy@xxxxxxxxxx>
branch nick: trunk
timestamp: Fri 2012-06-22 16:46:44 +0200
message:
Avoid deadlocks when changing user matchings
modified:
changelog.txt
dcpp/AdcHub.cpp
dcpp/AdcHub.h
dcpp/BufferedSocket.cpp
dcpp/BufferedSocket.h
dcpp/BufferedSocketListener.h
dcpp/Client.cpp
dcpp/Client.h
dcpp/DownloadManager.cpp
dcpp/DownloadManager.h
dcpp/NmdcHub.cpp
dcpp/NmdcHub.h
dcpp/UserConnection.cpp
dcpp/UserConnection.h
dcpp/UserMatchManager.cpp
dcpp/UserMatchManager.h
win32/UserInfoBase.cpp
win32/UserInfoBase.h
--
lp:dcplusplus
https://code.launchpad.net/~dcplusplus-team/dcplusplus/trunk
Your team Dcplusplus-team is subscribed to branch lp:dcplusplus.
To unsubscribe from this branch go to https://code.launchpad.net/~dcplusplus-team/dcplusplus/trunk/+edit-subscription
=== modified file 'changelog.txt'
--- changelog.txt 2012-06-21 18:37:35 +0000
+++ changelog.txt 2012-06-22 14:46:44 +0000
@@ -18,6 +18,7 @@
* [L#745162] Fix upload log format for partial lists (emtee)
* Fix GUI problems in a download attempt of a public hublist with invalid address (emtee)
* Fix unsuccessful HTTP redirections (emtee)
+* [L#1016205] Avoid deadlocks when changing user matchings (poy)
-- 0.799 2012-05-05 --
* Add icons (iceman50)
=== modified file 'dcpp/AdcHub.cpp'
--- dcpp/AdcHub.cpp 2012-06-21 18:52:47 +0000
+++ dcpp/AdcHub.cpp 2012-06-22 14:46:44 +0000
@@ -125,7 +125,7 @@
}
void AdcHub::clearUsers() {
- SIDMap tmp;
+ decltype(users) tmp;
{
Lock l(cs);
users.swap(tmp);
@@ -1106,4 +1106,13 @@
}
}
+OnlineUserList AdcHub::getUsers() const {
+ Lock l(cs);
+ OnlineUserList ret;
+ ret.reserve(users.size());
+ for(auto& i: users)
+ ret.push_back(i.second);
+ return ret;
+}
+
} // namespace dcpp
=== modified file 'dcpp/AdcHub.h'
--- dcpp/AdcHub.h 2012-05-15 23:26:22 +0000
+++ dcpp/AdcHub.h 2012-06-22 14:46:44 +0000
@@ -77,18 +77,11 @@
friend class Identity;
AdcHub(const string& aHubURL, bool secure);
-
- AdcHub(const AdcHub&);
- AdcHub& operator=(const AdcHub&);
virtual ~AdcHub();
- /** Map session id to OnlineUser */
- typedef unordered_map<uint32_t, OnlineUser*> SIDMap;
- typedef SIDMap::iterator SIDIter;
-
bool oldPassword;
Socket udp;
- SIDMap users;
+ unordered_map<uint32_t, OnlineUser*> users; /** Map session id to OnlineUser */
StringMap lastInfoMap;
mutable CriticalSection cs;
@@ -141,6 +134,7 @@
virtual void on(Second, uint64_t aTick) noexcept;
+ OnlineUserList getUsers() const;
};
} // namespace dcpp
=== modified file 'dcpp/BufferedSocket.cpp'
--- dcpp/BufferedSocket.cpp 2012-03-22 19:20:44 +0000
+++ dcpp/BufferedSocket.cpp 2012-06-22 14:46:44 +0000
@@ -444,8 +444,8 @@
if(p.first == SHUTDOWN) {
return false;
- } else if(p.first == UPDATED) {
- fire(BufferedSocketListener::Updated());
+ } else if(p.first == ASYNC_CALL) {
+ static_cast<CallData*>(p.second.get())->f();
continue;
}
@@ -522,7 +522,7 @@
}
void BufferedSocket::addTask(Tasks task, TaskData* data) {
- dcassert(task == DISCONNECT || task == SHUTDOWN || task == UPDATED || sock.get());
+ dcassert(task == DISCONNECT || task == SHUTDOWN || sock.get());
tasks.push_back(make_pair(task, unique_ptr<TaskData>(data))); taskSem.signal();
}
=== modified file 'dcpp/BufferedSocket.h'
--- dcpp/BufferedSocket.h 2012-01-13 20:55:20 +0000
+++ dcpp/BufferedSocket.h 2012-06-22 14:46:44 +0000
@@ -34,6 +34,7 @@
namespace dcpp {
using std::deque;
+using std::function;
using std::pair;
using std::unique_ptr;
@@ -98,8 +99,8 @@
/** Send the file f over this socket. */
void transmitFile(InputStream* f) { Lock l(cs); addTask(SEND_FILE, new SendFileInfo(f)); }
- /** Send an updated signal to all listeners */
- void updated() { Lock l(cs); addTask(UPDATED, 0); }
+ /** Call a function from the socket's thread. */
+ void callAsync(function<void ()> f) { Lock l(cs); addTask(ASYNC_CALL, new CallData(f)); }
void disconnect(bool graceless = false) noexcept { Lock l(cs); if(graceless) disconnecting = true; addTask(DISCONNECT, 0); }
@@ -115,7 +116,7 @@
SEND_FILE,
SHUTDOWN,
ACCEPTED,
- UPDATED
+ ASYNC_CALL
};
enum State {
@@ -139,6 +140,10 @@
SendFileInfo(InputStream* stream_) : stream(stream_) { }
InputStream* stream;
};
+ struct CallData : public TaskData {
+ CallData(function<void ()> f) : f(f) { }
+ function<void ()> f;
+ };
BufferedSocket(char aSeparator, bool v4only);
=== modified file 'dcpp/BufferedSocketListener.h'
--- dcpp/BufferedSocketListener.h 2012-01-13 20:55:20 +0000
+++ dcpp/BufferedSocketListener.h 2012-06-22 14:46:44 +0000
@@ -39,7 +39,6 @@
typedef X<5> ModeChange;
typedef X<6> TransmitDone;
typedef X<7> Failed;
- typedef X<8> Updated;
virtual void on(Connecting) noexcept { }
virtual void on(Connected) noexcept { }
@@ -49,7 +48,6 @@
virtual void on(ModeChange) noexcept { }
virtual void on(TransmitDone) noexcept { }
virtual void on(Failed, const string&) noexcept { }
- virtual void on(Updated) noexcept { }
};
} // namespace dcpp
=== modified file 'dcpp/Client.cpp'
--- dcpp/Client.cpp 2012-06-21 18:52:47 +0000
+++ dcpp/Client.cpp 2012-06-22 14:46:44 +0000
@@ -206,6 +206,11 @@
counts[COUNT_NORMAL].load(), counts[COUNT_REGISTERED].load(), counts[COUNT_OP].load()));
}
+void Client::updateUsers() {
+ // this is a public call, can come from any thread. bring it to this hub's thread.
+ if(sock) sock->callAsync([this] { auto users = getUsers(); updated(users); });
+}
+
void Client::updated(OnlineUser& user) {
UserMatchManager::getInstance()->match(user);
=== modified file 'dcpp/Client.h'
--- dcpp/Client.h 2012-05-24 17:47:25 +0000
+++ dcpp/Client.h 2012-06-22 14:46:44 +0000
@@ -21,15 +21,16 @@
#include "compiler.h"
+#include "atomic.h"
+#include "BufferedSocketListener.h"
+#include "ClientListener.h"
#include "forward.h"
-
#include "HubSettings.h"
+#include "OnlineUser.h"
#include "Speaker.h"
-#include "BufferedSocketListener.h"
#include "TimerManager.h"
-#include "ClientListener.h"
-#include "OnlineUser.h"
-#include "atomic.h"
+
+#include <boost/noncopyable.hpp>
namespace dcpp {
@@ -38,7 +39,8 @@
public Speaker<ClientListener>,
public BufferedSocketListener,
protected TimerManagerListener,
- public HubSettings
+ public HubSettings,
+ private boost::noncopyable
{
public:
virtual void connect();
@@ -71,8 +73,8 @@
const string& getIp() const { return ip; }
string getIpPort() const { return getIp() + ':' + port; }
- void updated(OnlineUser& user);
- void updated(OnlineUserList& users);
+ /** Send a ClientListener::Updated signal for every connected user. */
+ void updateUsers();
static string getCounts();
@@ -128,6 +130,9 @@
void updateCounts(bool aRemove);
void updateActivity() { lastActivity = GET_TICK(); }
+ void updated(OnlineUser& user);
+ void updated(OnlineUserList& users);
+
/** Reload details from favmanager or settings */
void reloadSettings(bool updateNick);
/// Get the external IP the user has defined for this hub, if any.
@@ -137,6 +142,7 @@
// TimerManagerListener
virtual void on(Second, uint64_t aTick) noexcept;
+
// BufferedSocketListener
virtual void on(Connecting) noexcept { fire(ClientListener::Connecting(), this); }
virtual void on(Connected) noexcept;
@@ -144,10 +150,9 @@
virtual void on(Failed, const string&) noexcept;
virtual bool v4only() const = 0;
+
private:
-
- Client(const Client&);
- Client& operator=(const Client&);
+ virtual OnlineUserList getUsers() const = 0;
string hubUrl;
string address;
=== modified file 'dcpp/DownloadManager.cpp'
--- dcpp/DownloadManager.cpp 2012-06-21 18:52:47 +0000
+++ dcpp/DownloadManager.cpp 2012-06-22 14:46:44 +0000
@@ -106,12 +106,24 @@
Lock l(cs);
for(auto uc: idlers) {
if(uc->getUser() == user) {
- uc->updated();
+ uc->callAsync([this, uc] { revive(uc); });
return;
}
}
}
+void DownloadManager::revive(UserConnection* uc) {
+ {
+ Lock l(cs);
+ auto i = find(idlers.begin(), idlers.end(), uc);
+ if(i == idlers.end())
+ return;
+ idlers.erase(i);
+ }
+
+ checkDownloads(uc);
+}
+
void DownloadManager::addConnection(UserConnectionPtr conn) {
if(!conn->isSet(UserConnection::FLAG_SUPPORTS_TTHF) || !conn->isSet(UserConnection::FLAG_SUPPORTS_ADCGET)) {
// Can't download from these...
@@ -432,18 +444,6 @@
aSource->disconnect();
}
-void DownloadManager::on(UserConnectionListener::Updated, UserConnection* aSource) noexcept {
- {
- Lock l(cs);
- auto i = find(idlers.begin(), idlers.end(), aSource);
- if(i == idlers.end())
- return;
- idlers.erase(i);
- }
-
- checkDownloads(aSource);
-}
-
void DownloadManager::fileNotAvailable(UserConnection* aSource) {
if(aSource->getState() != UserConnection::STATE_SND) {
dcdebug("DM::fileNotAvailable Invalid state, disconnecting");
=== modified file 'dcpp/DownloadManager.h'
--- dcpp/DownloadManager.h 2012-01-13 20:55:20 +0000
+++ dcpp/DownloadManager.h 2012-06-22 14:46:44 +0000
@@ -78,6 +78,7 @@
virtual ~DownloadManager();
void checkDownloads(UserConnection* aConn);
+ void revive(UserConnection* uc);
void startData(UserConnection* aSource, int64_t start, int64_t newSize, bool z);
void endData(UserConnection* aSource);
@@ -89,7 +90,6 @@
virtual void on(ProtocolError, UserConnection* aSource, const string& aError) noexcept { onFailed(aSource, aError); }
virtual void on(MaxedOut, UserConnection*) noexcept;
virtual void on(FileNotAvailable, UserConnection*) noexcept;
- virtual void on(Updated, UserConnection*) noexcept;
virtual void on(AdcCommand::SND, UserConnection*, const AdcCommand&) noexcept;
virtual void on(AdcCommand::STA, UserConnection*, const AdcCommand&) noexcept;
=== modified file 'dcpp/NmdcHub.cpp'
--- dcpp/NmdcHub.cpp 2012-06-08 15:27:48 +0000
+++ dcpp/NmdcHub.cpp 2012-06-22 14:46:44 +0000
@@ -128,7 +128,7 @@
}
void NmdcHub::clearUsers() {
- NickMap u2;
+ decltype(users) u2;
{
Lock l(cs);
@@ -1017,4 +1017,13 @@
}
}
+OnlineUserList NmdcHub::getUsers() const {
+ Lock l(cs);
+ OnlineUserList ret;
+ ret.reserve(users.size());
+ for(auto& i: users)
+ ret.push_back(i.second);
+ return ret;
+}
+
} // namespace dcpp
=== modified file 'dcpp/NmdcHub.h'
--- dcpp/NmdcHub.h 2012-05-23 22:05:22 +0000
+++ dcpp/NmdcHub.h 2012-06-22 14:46:44 +0000
@@ -67,10 +67,7 @@
mutable CriticalSection cs;
- typedef unordered_map<string, OnlineUser*, noCaseStringHash, noCaseStringEq> NickMap;
- typedef NickMap::iterator NickIter;
-
- NickMap users;
+ unordered_map<string, OnlineUser*, noCaseStringHash, noCaseStringEq> users;
int supportFlags;
@@ -92,10 +89,6 @@
NmdcHub(const string& aHubURL);
virtual ~NmdcHub();
- // Dummy
- NmdcHub(const NmdcHub&);
- NmdcHub& operator=(const NmdcHub&);
-
void clearUsers();
void onLine(const string& aLine) noexcept;
@@ -132,6 +125,7 @@
virtual void on(Line, const string& l) noexcept;
virtual void on(Failed, const string&) noexcept;
+ OnlineUserList getUsers() const;
};
} // namespace dcpp
=== modified file 'dcpp/UserConnection.cpp'
--- dcpp/UserConnection.cpp 2012-06-21 18:52:47 +0000
+++ dcpp/UserConnection.cpp 2012-06-22 14:46:44 +0000
@@ -212,10 +212,6 @@
fire(UserConnectionListener::TransmitDone(), this);
}
-void UserConnection::on(Updated) noexcept {
- fire(UserConnectionListener::Updated(), this);
-}
-
void UserConnection::on(Failed, const string& aLine) noexcept {
setState(STATE_UNCONNECTED);
fire(UserConnectionListener::Failed(), this, aLine);
=== modified file 'dcpp/UserConnection.h'
--- dcpp/UserConnection.h 2012-01-13 20:55:20 +0000
+++ dcpp/UserConnection.h 2012-06-22 14:46:44 +0000
@@ -126,7 +126,8 @@
void connect(const string& aServer, const string& aPort, const string& localPort, const BufferedSocket::NatRoles natRole);
void accept(const Socket& aServer);
- void updated() { if(socket) socket->updated(); }
+ template<typename F>
+ void callAsync(F f) { if(socket) socket->callAsync(f); }
void disconnect(bool graceless = false) { if(socket) socket->disconnect(graceless); }
void transmitFile(InputStream* f) { socket->transmitFile(f); }
@@ -212,7 +213,6 @@
virtual void on(ModeChange) noexcept;
virtual void on(TransmitDone) noexcept;
virtual void on(Failed, const string&) noexcept;
- virtual void on(Updated) noexcept;
};
} // namespace dcpp
=== modified file 'dcpp/UserMatchManager.cpp'
--- dcpp/UserMatchManager.cpp 2012-06-18 15:56:01 +0000
+++ dcpp/UserMatchManager.cpp 2012-06-22 14:46:44 +0000
@@ -51,8 +51,8 @@
const_cast<UserMatches&>(list) = move(newList);
// refresh user matches.
- for(auto& i: cm->getOnlineUsers()) {
- i.second->getClient().updated(*i.second);
+ for(auto i: cm->getClients()) {
+ i->updateUsers();
}
}
@@ -89,6 +89,18 @@
}
void UserMatchManager::ignoreChat(const HintedUser& user, bool ignore) {
+ setList(ignoreChatImpl(list, user, ignore));
+}
+
+void UserMatchManager::ignoreChat(const HintedUserList& users, bool ignore) {
+ auto newList = list;
+ for(auto& user: users) {
+ newList = ignoreChatImpl(newList, user, ignore);
+ }
+ setList(move(newList));
+}
+
+UserMatchManager::UserMatches UserMatchManager::ignoreChatImpl(const UserMatches& list, const HintedUser& user, bool ignore) {
auto nick = ClientManager::getInstance()->getNicks(user)[0];
UserMatch matcher;
@@ -132,7 +144,7 @@
}
newList.insert(newList.begin(), std::move(matcher));
- setList(std::move(newList));
+ return newList;
}
void UserMatchManager::on(SettingsManagerListener::Load, SimpleXML& xml) noexcept {
=== modified file 'dcpp/UserMatchManager.h'
--- dcpp/UserMatchManager.h 2012-01-13 20:55:20 +0000
+++ dcpp/UserMatchManager.h 2012-06-22 14:46:44 +0000
@@ -44,6 +44,7 @@
void match(OnlineUser& user) const;
void ignoreChat(const HintedUser& user, bool ignore);
+ void ignoreChat(const HintedUserList& users, bool ignore);
private:
friend class Singleton<UserMatchManager>;
@@ -53,6 +54,8 @@
UserMatchManager();
virtual ~UserMatchManager();
+ static UserMatches ignoreChatImpl(const UserMatches& list, const HintedUser& user, bool ignore);
+
void on(SettingsManagerListener::Load, SimpleXML& xml) noexcept;
void on(SettingsManagerListener::Save, SimpleXML& xml) noexcept;
};
=== modified file 'win32/UserInfoBase.cpp'
--- win32/UserInfoBase.cpp 2012-05-30 17:28:37 +0000
+++ win32/UserInfoBase.cpp 2012-06-22 14:46:44 +0000
@@ -80,6 +80,10 @@
UserMatchManager::getInstance()->ignoreChat(user, ignore);
}
+void UserInfoBase::ignoreChat(const HintedUserList& users, bool ignore) {
+ UserMatchManager::getInstance()->ignoreChat(users, ignore);
+}
+
tstring UserInfoBase::getTooltip() const {
static const size_t maxChars = 100; // max chars per tooltip line
=== modified file 'win32/UserInfoBase.h'
--- win32/UserInfoBase.h 2012-06-08 15:27:48 +0000
+++ win32/UserInfoBase.h 2012-06-22 14:46:44 +0000
@@ -47,6 +47,7 @@
virtual void removeFromQueue();
virtual void connectFav(TabViewPtr);
virtual void ignoreChat(bool ignore);
+ static void ignoreChat(const HintedUserList& users, bool ignore);
tstring getTooltip() const;
@@ -127,7 +128,10 @@
handleUserFunction([&](UserInfoBase* u) { u->connectFav(parent); });
}
void handleIgnoreChat(bool ignore) {
- handleUserFunction([ignore](UserInfoBase* u) { u->ignoreChat(ignore); });
+ // group multiple ignore calls into a single one.
+ HintedUserList users;
+ handleUserFunction([&users](UserInfoBase* u) { users.push_back(u->getUser()); });
+ UserInfoBase::ignoreChat(users, ignore);
}
void appendUserItems(TabViewPtr parent, Menu* menu, bool defaultIsGetList = true, bool includeSendPM = true) {
