[Adium-devl] Severe Structural AIChat-Issue in Adium
soc at monitzer.com
Thu Jul 26 21:22:20 UTC 2007
On Jul 26, 2007, at 22:59, Evan Schoenberg wrote:
> On Jul 19, 2007, at 12:12 PM, Andreas Monitzer wrote:
>> convLookupFromChat() sets up the necessary information for
>> lilbpurple, and calls serv_join_chat (this one doesn't return
>> anything). This is all that has to be done from Adium.
> It seems like here we have the first problem. serv_join_chat()
> appears not to be able to return the PurpleConversation, because it
> may not exist for some time to come - at least some of the prpls do
> asynchronous joins and create the conversation on success. I think
> that, for starters:
> * serv_join_chat() needs some way of indicating an error. This
> could be a return value or an error-returned-by-reference.
The problem is, that it doesn't know yet if there's an error. Only
the server knows, and it takes at least one read() call to get this
> * I say the latter because I think the return value should be a
> handle which can be used to identify the PurpleConversation once it
> is created.
That'd be another solution.
> There is already an integer id used when passing to
> serv_got_joined_chat - it appears this is unique per prpl. Perhaps
> returning this and making use of this could be a good direction?
>> Now, libpurple joins the chat, and calls adiumPurpleConvCreate() on
>> success. Note that libpurple doesn't call anything in Adium when it
>> doesn't succeed. Since Adium already created the AIChat object, this
>> is the cause for bug #1874 (Adium thinks that there's already a
>> groupchat with that MUC, so it disallows the join).
> I think having already created the AIChat object is fine - the
> problem is that it's not destroyed on error. As mentioned above,
> serv_join_chat() should return an error if it fails immediately...
> but if there's a failure some other way, we still need to find
> out. What do you think about a serv_failed_joined_chat() or
> something like that which the prpl would call and which would, in
> turn, let us know that the join attempt failed?
The problem is that the libpurple API is reasonable here, Adium is
just utilizing it incorrectly. You will have a hard time explaining
to them why changing the way it works is needed.
One argument would be that libpurple shouldn't assume a certain code
design in the application, since the current design in Adium simply
doesn't support this way of handling things.
> I'm a bit confused at this point - are you saying you've found that
> 2 AIChat objects are created for every joined group chat?
Only on success. On failure, only the leaked one stays, the real
instance doesn't get created.
> I think, if so, that's a 1.1/trunk regression, though I'm not
> positive - and in that case I may have caused that while working on
> fixing MSN group chats.
The corresponding ticket was created two years ago, so I don't think
this is a new one.
> Does this case only occur if there has been a previous failure, or
> does it happen every time?
It happens every time. Usually it's not a problem (except for the
memory leak of course), but there are certain cases where the user
notices it, as described in those tickets.
More information about the Adium-devl