[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kopete-devel
Subject:    Re: [Kopete-devel] MSNSocket
From:       Martijn Klingens <klingens () kde ! org>
Date:       2002-11-14 15:21:51
[Download RAW message or body]

On Wednesday 13 November 2002 15:16, Olivier Goffart wrote:
> Martijn wrote:
> >Hmm, maybe we should use Qt events instead of signals here, because then
> > we enter the protocol through the message loop. I have never done such a
> > thing though and I foresee quite a lot of other problems. All in all that
> > 'solution' might be worse than our current problem.
>
> mmmh... i don't see how to do that.
> that mean MSNSocket must herits from KExtendedSocket.

No, not quite. We can define our own events and use KApplication::postEvent() 
and friends to put them at the end of the queue. But I don't know how it 
works exactly and I'm personally not too fond of this approach. I just 
mentioned it as an alternative.

> >The only viable alternative is using deleteLater, which should always work
> >correct.
>
> exepted if the parent object is destroyed before

When does this happen? I'm more and more convinced that we're not trying to 
fix a few bugs, but trying to work around a severe design problem that 
happens to cause bugs instead. I'd rather fix it at the design level, but I 
am a bit unsure where the real problem is. Could you explain which objects 
interact and where the memory management goes berserk? (Alternatively, could 
you wait until I have time to work on Kopete myself again, so I can analyse 
the problem? ;-)

> >Oh, wait, the problem is that the destructor _has_ to be called because it
> >does not only clean up memory, but also do other processing, like
> > disconnect, right? If so, just move that small part out of the destructor
> > into another method and call that method directly before deleteLater.
>
> Do not forget that the socket can be closed by the other side. that's why
> we need to delete him in a close event

A closed socket doesn't delete itself I hope, we still need to clean up and 
free memory.

> >Try adding a user :-)
> >
> >You add to both the FL and AL lists, and this _has_ to be done through the
> >queue or you will suffer. And since it never hurts if we follow the
> > protocol correctly it's best to keep it for everything.
>
> Ok....
> no problem when i add contact (maybe because i have moved the ADD AL
> command to slotContactAdded a time ago)
> But there is a problem with block/unblock user. I can fix it easier without
> the queue (moving the ADD command to slotContactRemoved)
>
> is there any other place where two command are sent in the same time?

Not sure. I wanted to _avoid_ the problem by introducing the queue. Because 
it's timing related it's technically possible that otherwise things work for 
you or me but not for others. The queue was meant to make things more robust.

Could you paste a bit of the communication with the server (sent messages and 
received responses) where the queue breaks?

> >That said, I don't understand why you needed an 'ugly' hack to make it
> > work. What was the hack about? Do you have a diff or so?
>
> see MSNSwitchBoardSocket::slotTypingMsg or
> MSNSwitchBoardSocket::slotReadMessage
>
> m_lastId++;  //FIXME:  there is no ACK for this command ; without
>     // m_lastId++, future messages are queued  (MSNSocket::m_lastId
>     // should be private)

If the queue is unnecessary we might do it. I am hesitant to remove it though. 
What do you think? Is it really safe to remove the queuing and don't we 
introduce other problems by doing so?

Maybe you could comment out the if() at the end of MSNSocket::sendCommand and 
calling writeBlock directly to force bypassing the queue. If no problems pop 
up in the coming weeks by doing so we can remove the queuing code altogether 
and otherwise we need to fix the queue. Is that reasonable?

> and a 3th argument tegen the queue: sometimes the server send several
> command with the same id

That shouldn't be a problem AFAICS. But try my above suggestion first I'd say.
-- 
Martijn

_______________________________________________
Kopete-devel mailing list
Kopete-devel@mail.kde.org
http://mail.kde.org/mailman/listinfo/kopete-devel
[prev in list] [next in list] [prev in thread] [next in thread]