[Webkit-unassigned] [Bug 61287] [UNIX] Don't use SOCK_DGRAM in socketpair()

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-05-26 00:18:04 PST ---
(In reply to comment #11)
> (In reply to comment #0)
> > While implemeting the plugin process for the GTK port I noticed that when closing a page containing a plugin, the web process got blocked waiting for a sync message reply which was never sent because the plugin process was about to die. 
> 
> So the problem here is not the usage of SOCK_DGRAM per se. AFAICS the real problem is that the web process or UI process is not able to observe plugin process crash.

Not exactly, we already detect when the child process crashes, and it works, the problem is when the child process closes the connection and finishes. In the case of the plugin process there's a timeout to finish the process. The plugin process closes the connection and the web process doesn't notice it, so it gets blocked waiting for the sync reply to DestroyPlugin message. If the plugin process finishes before the sync timeout, the web process is notified and it wakes up, otherwhise it wakes up after the sync timout. Either way the web process gets blocked for a while. So we want to be notified as soon as the connection is closed by the other end, no matter whether the process finishes or not.

> The original intention of the of using SOCK_DGRAM was two-fold:
>  - it guarantees that the message is not buffered at any point redundantly (minimum latency)
>  - it simplifies implementation greatly

Yeah, I agree. 

> The original solution to the real problem in this bug report was to listen the child process crash by other means.
> 
> The original reason of using SOCK_DGRAM instead of SOCK_SEQPACKET was to avoid discussion about Linux specificness of the original patch.

Yes, DGRAM and STREAM are more portable. SEQPACKET is exactly what we want, it uses DGRAM functions internally and we get notified when the other end closes the connection

> I would advice not using SOCK_STREAM. The main problem I see is the lack of guarantees about buffering and need for receiver to preserve buffers, split messages correctly etc. Also the implementation gets quite complex, as you need to buffer the file descriptors. Anecdotal evidence of this complexity might be for example that the suggested patch leaks file descriptors when connection is closed while the fds are in buffer. (AFAICS from very quick inspection, sorry if I'm wrong). 

oops :-P

> Also remember that Mac implementation is dgram-based, so it is very much easier to achieve similar performance characteristics, avoid unnecessary kludging in common parts and maybe even code reuse.

Agree.

> My suggestion would be to (not that it should matter in this point):
>  1. Use SOCK_SEQPACKET on Linux, observe crashes with recvmsg == 0
>  2. ifdef Non-Linux, use platform specific way of observing child process crash
>  3. rename this bug
> 
> 1. is needed only if you are perfectionist, otherwise only 2. suffices.
> 
> To do 2. cross-platform, you can for example open dummy SOCK_STREAM during fork and make that communicate the crash of child process. I would expect this to have much less overhead than redundant buffering of should-not-be-buffered messages.

Wouldn't polling another socket be a performance issue too? Anyway, I'll try that way, using SEQPACKET when available and falling back to polling a dummy socket. Thank you very much for the comments. 

Btw, could you also take a look at patch attached to bug #60621, please?

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