Skip to content

Commit

Permalink
Revert "Rewrite the JOIN channel logic, dropping MaxJoins"
Browse files Browse the repository at this point in the history
This reverts commit db7c47f.

Too many joined channels at once started to cause disconnect because of
"Max SendQ Exceeded", which is not much better than previous Flood.

Now MaxJoins is 0 by default, which preserves the current behavior of
joining all channels at once. If someone experiences those disconnects
due to SendQ, they can tune MaxJoins.

Fix #329

Conflicts:
	include/znc/User.h
	modules/controlpanel.cpp
	modules/webadmin.cpp
	src/User.cpp
  • Loading branch information
DarthGandalf committed Jun 9, 2013
1 parent 516b3fb commit a06b649
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 30 deletions.
1 change: 1 addition & 0 deletions include/znc/IRCNetwork.h
Expand Up @@ -72,6 +72,7 @@ class CIRCNetwork {
bool AddChan(const CString& sName, bool bInConfig);
bool DelChan(const CString& sName);
void JoinChans();
void JoinChans(std::set<CChan*>& sChans);

const CString& GetChanPrefixes() const { return m_sChanPrefixes; };
void SetChanPrefixes(const CString& s) { m_sChanPrefixes = s; };
Expand Down
3 changes: 3 additions & 0 deletions include/znc/User.h
Expand Up @@ -124,6 +124,7 @@ class CUser {
void SetTimestampPrepend(bool b) { m_bPrependTimestamp = b; }
void SetTimezone(const CString& s) { m_sTimezone = s; }
void SetJoinTries(unsigned int i) { m_uMaxJoinTries = i; }
void SetMaxJoins(unsigned int i) { m_uMaxJoins = i; }
void SetSkinName(const CString& s) { m_sSkinName = s; }
void SetMaxNetworks(unsigned int i) { m_uMaxNetworks = i; }
// !Setters
Expand Down Expand Up @@ -165,6 +166,7 @@ class CUser {
unsigned long long BytesRead() const { return m_uBytesRead; }
unsigned long long BytesWritten() const { return m_uBytesWritten; }
unsigned int JoinTries() const { return m_uMaxJoinTries; }
unsigned int MaxJoins() const { return m_uMaxJoins; }
CString GetSkinName() const;
unsigned int MaxNetworks() const { return m_uMaxNetworks; }
// !Getters
Expand Down Expand Up @@ -212,6 +214,7 @@ class CUser {
unsigned long long m_uBytesWritten;
unsigned int m_uMaxJoinTries;
unsigned int m_uMaxNetworks;
unsigned int m_uMaxJoins;
CString m_sSkinName;

CModules* m_pModules;
Expand Down
8 changes: 8 additions & 0 deletions modules/controlpanel.cpp
Expand Up @@ -59,6 +59,7 @@ class CAdminMod : public CModule {
{"AutoClearChanBuffer", boolean},
{"Password", str},
{"JoinTries", integer},
{"MaxJoins", integer},
{"Timezone", str},
{"Admin", boolean},
{"AppendTimestamp", boolean},
Expand Down Expand Up @@ -177,6 +178,8 @@ class CAdminMod : public CModule {
PutModule("KeepBuffer = " + CString(!pUser->AutoClearChanBuffer())); // XXX compatibility crap, added in 0.207
else if (sVar == "autoclearchanbuffer")
PutModule("AutoClearChanBuffer = " + CString(pUser->AutoClearChanBuffer()));
else if (sVar == "maxjoins")
PutModule("MaxJoins = " + CString(pUser->MaxJoins()));
else if (sVar == "jointries")
PutModule("JoinTries = " + CString(pUser->JoinTries()));
else if (sVar == "timezone")
Expand Down Expand Up @@ -292,6 +295,11 @@ class CAdminMod : public CModule {
pUser->SetPass(sHash, CUser::HASH_DEFAULT, sSalt);
PutModule("Password has been changed!");
}
else if (sVar == "maxjoins") {
unsigned int i = sValue.ToUInt();
pUser->SetMaxJoins(i);
PutModule("MaxJoins = " + CString(pUser->MaxJoins()));
}
else if (sVar == "jointries") {
unsigned int i = sValue.ToUInt();
pUser->SetJoinTries(i);
Expand Down
5 changes: 5 additions & 0 deletions modules/data/webadmin/tmpl/add_edit_user.tmpl
Expand Up @@ -261,6 +261,11 @@
<input type="number" name="jointries" value="<? VAR JoinTries ?>" class="third" min="0"
title="This defines how often ZNC tries to join, if the first join failed, e.g. due to channel mode +i/+k or if you're banned." />
</div>
<div class="subsection">
<div class="inputlabel">Max Joins:</div>
<input type="number" name="maxjoins" value="<? VAR MaxJoins ?>" class="third" min="0"
title="How many channels are joined in one JOIN command. 0 is unlimited (default). Set to small positive value if you get disconnected with `Max SendQ Exceeded'"/>
</div>
<div class="subsection">
<div class="inputlabel">Max IRC Networks Number:</div>
<input type="number" name="maxnetworks" value="<? VAR MaxNetworks ?>" class="third" min="0"
Expand Down
2 changes: 2 additions & 0 deletions modules/webadmin.cpp
Expand Up @@ -269,6 +269,7 @@ class CWebAdminMod : public CModule {
pNewUser->SetTimestampPrepend(WebSock.GetParam("prependtimestamp").ToBool());
pNewUser->SetTimezone(WebSock.GetParam("timezone"));
pNewUser->SetJoinTries(WebSock.GetParam("jointries").ToUInt());
pNewUser->SetMaxJoins(WebSock.GetParam("maxjoins").ToUInt());

if (spSession->IsAdmin()) {
pNewUser->SetDenyLoadMod(WebSock.GetParam("denyloadmod").ToBool());
Expand Down Expand Up @@ -1073,6 +1074,7 @@ class CWebAdminMod : public CModule {
Tmpl["Timezone"] = pUser->GetTimezone();
Tmpl["JoinTries"] = CString(pUser->JoinTries());
Tmpl["MaxNetworks"] = CString(pUser->MaxNetworks());
Tmpl["MaxJoins"] = CString(pUser->MaxJoins());

const set<CString>& ssAllowedHosts = pUser->GetAllowedHosts();
for (set<CString>::const_iterator it = ssAllowedHosts.begin(); it != ssAllowedHosts.end(); ++it) {
Expand Down
75 changes: 48 additions & 27 deletions src/IRCNetwork.cpp
Expand Up @@ -668,46 +668,67 @@ bool CIRCNetwork::DelChan(const CString& sName) {
}

void CIRCNetwork::JoinChans() {
bool bHaveKey = false;
size_t joinLength = 4; // join
CString sChannels, sKeys;
// Avoid divsion by zero, it's bad!
if (m_vChans.empty())
return;

// We start at a random offset into the channel list so that if your
// first 3 channels are invite-only and you got MaxJoins == 3, ZNC will
// still be able to join the rest of your channels.
unsigned int start = rand() % m_vChans.size();
unsigned int uJoins = m_pUser->MaxJoins();
set<CChan*> sChans;
for (unsigned int a = 0; a < m_vChans.size(); a++) {
unsigned int idx = (start + a) % m_vChans.size();
CChan* pChan = m_vChans[idx];
if (!pChan->IsOn() && !pChan->IsDisabled()) {
if (!JoinChan(pChan))
continue;

for (vector<CChan*>::iterator it = m_vChans.begin(); it != m_vChans.end(); ++it) {
CChan *pChan = *it;
sChans.insert(pChan);

if (pChan->IsOn() || pChan->IsDisabled() || !JoinChan(pChan)) {
continue;
// Limit the number of joins
if (uJoins != 0 && --uJoins == 0)
break;
}
}

while (!sChans.empty())
JoinChans(sChans);
}

size_t length = pChan->GetName().length() + pChan->GetKey().length() + 2; // +2 for either space or commas
void CIRCNetwork::JoinChans(set<CChan*>& sChans) {
CString sKeys, sJoin;
bool bHaveKey = false;
size_t uiJoinLength = strlen("JOIN ");

if ((joinLength + length) >= 510) {
// Sent what we got, and cleanup
PutIRC("JOIN " + sChannels + (bHaveKey ? (" " + sKeys) : ""));
while (!sChans.empty()) {
set<CChan*>::iterator it = sChans.begin();
const CString& sName = (*it)->GetName();
const CString& sKey = (*it)->GetKey();
size_t len = sName.length() + sKey.length();
len += 2; // two comma

sChannels = "";
sKeys = "";
joinLength = 4; // join
bHaveKey = false;
}
if (!sKeys.empty() && uiJoinLength + len >= 512)
break;

if (!sChannels.empty()) {
sChannels += ",";
if (!sJoin.empty()) {
sJoin += ",";
sKeys += ",";
}

if (!pChan->GetKey().empty()) {
uiJoinLength += len;
sJoin += sName;
if (!sKey.empty()) {
sKeys += sKey;
bHaveKey = true;
sKeys += pChan->GetKey();
}

sChannels += pChan->GetName();
joinLength += length;
sChans.erase(it);
}

if (!sChannels.empty()) {
PutIRC("JOIN " + sChannels + (bHaveKey ? (" " + sKeys) : ""));
}
if (bHaveKey)
PutIRC("JOIN " + sJoin + " " + sKeys);
else
PutIRC("JOIN " + sJoin);
}

bool CIRCNetwork::JoinChan(CChan* pChan) {
Expand Down
7 changes: 4 additions & 3 deletions src/User.cpp
Expand Up @@ -87,6 +87,7 @@ CUser::CUser(const CString& sUserName)
m_uBufferCount = 50;
m_uMaxJoinTries = 10;
m_bAutoClearChanBuffer = true;
m_uMaxJoins = 0;
m_bBeingDeleted = false;
m_sTimestampFormat = "[%H:%M:%S]";
m_bAppendTimestamp = false;
Expand Down Expand Up @@ -144,6 +145,7 @@ bool CUser::ParseConfig(CConfig* pConfig, CString& sError) {
TOption<unsigned int> UIntOptions[] = {
{ "jointries", &CUser::SetJoinTries },
{ "maxnetworks", &CUser::SetMaxNetworks },
{ "maxjoins", &CUser::SetMaxJoins },
};
size_t numUIntOptions = sizeof(UIntOptions) / sizeof(UIntOptions[0]);
TOption<bool> BoolOptions[] = {
Expand Down Expand Up @@ -189,9 +191,6 @@ bool CUser::ParseConfig(CConfig* pConfig, CString& sError) {

CString sValue;

// MaxJoins has been removed
pConfig->FindStringEntry("maxjoins", sValue);

CString sDCCLookupValue;
pConfig->FindStringEntry("dcclookupmethod", sDCCLookupValue);
if (pConfig->FindStringEntry("bouncedccs", sValue)) {
Expand Down Expand Up @@ -668,6 +667,7 @@ bool CUser::Clone(const CUser& User, CString& sErrorRet, bool bCloneNetworks) {
SetBufferCount(User.GetBufferCount(), true);
SetJoinTries(User.JoinTries());
SetMaxNetworks(User.MaxNetworks());
SetMaxJoins(User.MaxJoins());

// Allowed Hosts
m_ssAllowedHosts.clear();
Expand Down Expand Up @@ -863,6 +863,7 @@ CConfig CUser::ToConfig() {
config.AddKeyValuePair("Timezone", m_sTimezone);
config.AddKeyValuePair("JoinTries", CString(m_uMaxJoinTries));
config.AddKeyValuePair("MaxNetworks", CString(m_uMaxNetworks));
config.AddKeyValuePair("MaxJoins", CString(m_uMaxJoins));

// Allow Hosts
if (!m_ssAllowedHosts.empty()) {
Expand Down

0 comments on commit a06b649

Please sign in to comment.