[Webkit-unassigned] [Bug 115351] Make sure ResourceHandleSoup::platformSetDefersLoading() does not call ResourceHandleClient callbacks synchronously

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Andre Moreira Magalhaes <andrunko at gmail.com>  2013-04-29 09:34:49 PST ---
(In reply to comment #6)
> (In reply to comment #5)
> 
> > I am not sure why this is the case as the callbacks are already called asynchronously when there isn't a deferred result. What I see here is an inconsistency in the ResourceHandleSoup where it sometimes invokes the callbacks synchronously and sometimes asynchronously. The problem is not the gst player calling this from a different thread (WebCore methods are called from main thread), the problem is that the methods calling setDefersLoading is protected by a mutex and the same is true for the callbacks for data IIRC, thus invoking the data callback when calling setDefersLoading causes a deadlock in the gst source element.
> 
> Hrm. I misunderstood the problem originally, but I think I understand it now. The restriction that data flow needs to happen asynchronously after calling arbitrary methods on ResourceHandle does seem like an arbitrary requirement that's particular to the GStreamer backend. These kind of unspoken contracts make it hard to refactor ResourceHandle, for instance. It seems that the media code should make as few assumptions about how the ResourceHandle works as possible (within reason, of course).
> 
> The class that maintains the mutex should know when it has already acquired it or alternatively it could use recursive mutexes, if they aren't too expensive. I'm sympathetic to the desire to fix the problem here and the idea that undeferring loading should always trigger data asynchronously, but I think that should be a design choice and not part of the contract. Thus, this should probably be handled in the media layer.

Agree that this should be handled by the gstreamer backend itself. The problem is that the resourceHandle member itself needs to be protected by the mutex (to check it the pointer is still valid) as it can be unrefed from a different thread or something like that (I wrote this patch some time ago and don't remember the details). The same mutex is used here for both the resourceHandle and the data callback but I am sure there are other alternatives. In any case I believe this patch should be applied so that the setDefersLoading method always have the same behaviour, but this is up to you guys. Note that this only applies to the Soup resource handle.

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