[Webkit-unassigned] [Bug 74108] [Qt] Incorrect deletion of m_replyWrapper in QtNetworkReplyHandler::finish

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=74108





--- Comment #9 from Luiz Agostini <luiz at webkit.org>  2011-12-19 18:57:22 PST ---
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > Follow frame #11 back to #6 and you will see the QNetworkReplyWrapper being deleted in QNetworkReplyHander::finish. Unfortunately, the call to QNetworkReplyHandler::finish originated from the very same QNetworkReplyWrapper (QNetworkReplyWrapper::didReceiveFinished) that was deleted!! 
> > 
> > You mean that a QNetworkReplyWrapper object is been deleted in a SLOT connected to the finish signal of a QNetworkReply object. I do not see why it would be a problem because, as you said, the QNetworkReply object's destruction is handled properly.
> 
> No. I actually meant a QNetworkReplyWrapper object is getting deleted from a function that is contains within itself. IOW, the object is indvertantly getting deleted before the calling function can return.

Please, just look at the QNetworkReplyWrapper methods that appear in your backtrace to see that it is not a problem. The QNetworkReplyWrapper is been destroyed inside one of its methods, actually a slot connected to the signal finish of a QNetworkReply, but it is not a problembecause no member of the object and no method of that object is called after destroying it.

It is not "inadvertantly" and it will not cause any crash.

> 
> > > Nasty. No clue why this does not cause more crashes.
> > 
> > This does not cause crashes because the queue (QNetworkReplyHandlerCallQueue) makes it sure that all the methods will get called in proper sequence and that the QNetworkReplyWrapper object will not be used after been destroyed. The object gets destroyed inside of QNetworkReplyWrapper::didReceiveFinished only if the queue is empty.
> 
> Even if the Queue is not empty once you call m_queue->push from QNetworkReplyHandler, all the calls in the queue will immediately be executed because push calls flush. The only way to prevent ::flush from iterating over each function and executing them is through the explicit use of QueueLocker or directly locking the queue yourself. Unfortunately, the queue locking seems to be done correctly everywhere except in QNetworkReplyHandler::didReceiveFinished, where it is completely missing. Because of that QNetworkReplyHandler::finish is called from didReceiveFinished and hence the statement I made above. Why is there no queue locking done in didReceiveFinished as done in all the other places where QNetworkReplyHandler::finish was added to the queue ?

If the queue is flushing then the method been pushed will not be executed immediately, with or without the lock. If the queue is flushing then the method will wait in the queue until its turn.

The lock is used when it is needed to push more then one method in sequence to the queue or if it is needed to something else before flushing the queue. The locker is not been used in didReceiveFinished because it would not help at all (see that flush would be called by the locker destructor).

I see no mystery here.

> 
> > It is not clear for me now if the problem that you have is indeed related to the destruction of the QNetworkReplyWrapper. You mentioned that there is no problem related to the destruction of the QNetworkReply object as the bug description states. It seems to me now that both, bug title and bug description, are incorrect.
> 
> > Could you provide more information?
> 
> What more information can I provide other than the detailed backtrace I already provided ?

By looking at the bug title, bug description and any of the comments in this bug (including the backtrace that you provided), I am still seeing no obvious problem related to the QNetworkReplyWrapper and/or QNetworkReply destruction.

It would be very nice if you could provide a test case.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.