Autojoining jabber groupchat is broken in bitlbee-3.4

[faure@…]

Using bitlbee to connect to a company Jabber server, and using konversation as the GUI client, works rather well. Or rather, it did until 3.2.2.

In bitlbee 3.4, the auto_join functionality doesn't work anymore: bitlbee logs in, all the users appear in the bitlbee tab, but it doesn't join the channels, so one feels rather lonely :)

[wilmer]

Grr. So this is broken altogether then. :-( I was having issues with this while testing my Whatsapp code already and hadn't really investigated that more.

Let's see what it takes to track this down using git bisect or so..

[dx]

Uh, works for me? Never noticed any issues either

[Milian Wolff]

I have the same issue with the bitlbee 3.4 version supplied by archlinux. My IRC client (also Konversation) joins the group chats just fine (and I configured bitlbee to auto_join the channels), but the group chats only show me and root. Is there some log file or anything else we could provide you with to get this issue fixed?

[dx]

Does this happen after reconnecting? Or, more specifically, when your irc client is already joined to those channels by the time you use the "identify" command?

[Milian Wolff]

Yes this happens after reconnecting. And yes, I assume the identify gets send later.

[dx]

Did a bisect. ecbd22a87ca7bc2675e9368445d193c6c81bf3c1 is the commit that broke it, but i'm not sure if reverting is the best solution to this problem.

[Milian Wolff]

So, anything we could do to help this issue getting sorted out? Should I test with the commit reverted or anything else I could test for you?

[dx]

Well if you revert that commit locally, (git revert <commit>) it will most likely solve this particular issue for you. Personally I'd like to dig more.

[Renato Botelho <garga@…>]

This is really annoying to need to run a '/leave' and '/j #channel' every time I reconnect. After a quick look it seems that ic->flags already contain IRC_CHANNEL_JOINED set by the first call of irc_channel_auto_joins() done when it fist connect to bitlbee, before identify, and then, it doesn't re-join channels after user is identified

[wilmer]

So. Looking at this now, I'll see how to refactor this. I *think* the situation before this patch was better. But still not optimal.

I think it should just be possible to join the channel even when the account is offline, and BitlBee should just leave it for dead until the account goes online - then just tie the groupchat and the channel to each other. Shouldn't be *too* hard hopefully.

I'll just have to dig through the code a little bit to remember how that stuff's structured. Took me a while to even remember what IRC_CHANNEL_JOINED means officially - still not 100% sure.

[wilmer]

dx, did you actually experience any memory leakage without that change BTW?

The original code and comment did in fact match. However the double negative combined with the fact that continue is a C statement which in this case is the opposite of what "continue" in the comment means, doesn't really help. :-)

The point was to, while iterating through the chan list, skip (not auto-join) any channel that is not marked as auto-join in its settings, and that the user is not already in (due to IRC client reconnect).

I can't think off the top of my head of any actual memory leakage or so caused by that code. It's a little odd because the join here actually just means joining the IM chatroom to the IRC channel. The user is already joined, the second call to irc_channel_add_user should just get ignored. Which means a bit of a noisy join (it's nicer to fill the room with contacts while the user isn't watching) but meh.

So unless I've forgotten something, I'll revert this change and perhaps clarify the comment. Then we'll have more or less the behaviour I described in my previous comment, only difference being that it's not possible to join the channel in the time window in between settings being loaded (so #bitlbee then knows about the existence of the channel) and the account coming online. Meh.

[dx]

Replying to wilmer:

dx, did you actually experience any memory leakage without that change BTW?

Trac sucks and took the "git-committer" field as author. Refer to "git-author" instead - jgeboski wrote it.

I think we should figure out how to reproduce this "double auto-join".

[dx]

Okay here's some context to that commit: ​http://dump.dequis.org/i4XHw.txt (all the links still work)

tl;dr: While writing the twitter filter channels patch, there was a message duplication bug that started when disconnecting from bitlbee and rejoining, and the twitter prpl->chat_join() function was called twice. I think the memory leaks are theoretical.

Also god damn these double negatives are confusing.

After being confused for a long while I made this thing in google spreadsheets.

Reference: The white columns are and(not(A2), not(B2)) and or(not(A2), B2) respectively. The "join it?" columns are just a "NOT" of the white ones as an attempt to reduce confusion (as in, if "continue" is not run, it will join the channel)

It gets less confusing once you can ignore what the comment says.

So, I guess the old behavior makes sense if whatever set the IRC_CHANNEL_JOINED flag didn't call prpl->chat_join(). Or if that call was no-op.

I guess twitter doesn't really need a login before establishing a filter stream connection (old oauth credentials are okay), so if by the time the irc client sends JOIN bitlbee already set twitter to online, the first join happens. Small race condition there, I guess.

Also, it's silly that this function doesn't know the difference between "joined" and "unused".

[wilmer]

Funny. So the double call to chat_join() shouldn't happen for sure. Probably the right fix here would be to add another check here:

        if ((acc_s = set_getstr(&ic->set, "account")) &&
            (room = set_getstr(&ic->set, "room")) &&
            (acc = account_get(ic->irc->b, acc_s)) &&
            acc->ic && acc->prpl->chat_join) {

It's indeed a bit of a race. The autojoins are processed at imcb_connected() time, and "manual" (which includes automatic by IRC lient) joins are currently disallowed in between identify/config load time and login-start time instead of login-finish time.

So just adding a OPT_LOGGED_IN check should do the trick. That and then rolling back the previously mentioned change.

Trac sucks and took the "git-committer" field as author. Refer to "git-author" instead - jgeboski wrote it.

Grmbl, sorry, yeah, I remember you mentioning jgeboski did it but then I saw you as the author. Silly Trac indeed. I'm pretty sure Bzr did that better. :-P

[wilmer]

​https://github.com/bitlbee/bitlbee/commit/24de9fa0c79027094383aa0c247e48a4bc6753d3

Things should be back to normal starting from the upcoming nightly build!

[wilmer]

Just realised I never actually closed the ticket. Please reopen if this is still an issue, I believe it should be fixed in current head.

[faure@…]

Works great, thanks a lot for the fix.