[webkit-gtk] Sockets/file descriptors and the web extension

[Eric Williams]

Hello,

We've encountered an interesting bug in Eclipse SWT that involves the
web extension and sockets/file descriptors. Some background: in SWT we
implemented our own web extension to handle JS execution -- it doesn't
really do anything exciting beyond that.

The bug is as follows:

1) A user in Eclipse uses an Eclipse plugin to open a port on their
machine. The socket that's opened is owned by Eclipse/SWT
2) The user then hovers over a Java class in Eclipse, that subsequently
opens a browser popup which executes some JS. Thus Eclipse/SWT's WebKit
instance along with a web extension is created and loaded.
3) The user (using Eclipse/SWT) closes the port they opened in step 1).
4) The user tries to re-open that port again which fails with a
BindException -- socket in use/not available.

Some preliminary debugging shows that the web extension still holds onto
the socket/file descriptor used when opening the port in step 1 -- even
though the port has been closed. This causes the error in step 4. If no
web extension is created (i.e. step 2 is skipped), then no error from
step 4 happens.

Is this a bug? Or is it some sort of design decision made inside WebKit
itself? Anything to shed some light on this issue would be helpful. :)


Thanks,

[Michael Catanzaro]

Post by Eric Williams
Some preliminary debugging shows that the web extension still holds
onto the socket/file descriptor used when opening the port in step 1
-- even though the port has been closed. This causes the error in
step 4. If no web extension is created (i.e. step 2 is skipped), then
no error from step 4 happens.

I'm confused and suspect I've misunderstood the scenario. But: why does
WebKit have anything to do with a socket that you create in your own
code?

[Eric Williams]

Post by Michael Catanzaro

Post by Eric Williams
Some preliminary debugging shows that the web extension still holds
onto the socket/file descriptor used when opening the port in step 1
-- even though the port has been closed. This causes the error in step
4. If no web extension is created (i.e. step 2 is skipped), then no
error from step 4 happens.

I'm confused and suspect I've misunderstood the scenario. But: why does
WebKit have anything to do with a socket that you create in your own code?

This is kind of what I'm trying to get at: is it possible that WebKit
gets a copy of all open sockets somehow when creating an extension? If
this is completely outside the realm of possibility then I'll
investigate further.


Eric

[Adrian Perez de Castro]

Hello Eric,

Post by Eric Williams

Post by Michael Catanzaro

Post by Eric Williams
Some preliminary debugging shows that the web extension still holds
onto the socket/file descriptor used when opening the port in step 1
-- even though the port has been closed. This causes the error in step
4. If no web extension is created (i.e. step 2 is skipped), then no
error from step 4 happens.

I'm confused and suspect I've misunderstood the scenario. But: why does
WebKit have anything to do with a socket that you create in your own code?

This is kind of what I'm trying to get at: is it possible that WebKit
gets a copy of all open sockets somehow when creating an extension? If
this is completely outside the realm of possibility then I'll
investigate further.

Are you setting the close-on-exec flag [1] on the file descriptor? If not,
it may be as well that the file descriptor for the socket is being inherited
by the child processes (WebKitNetworProcess, WebKitWebProcess, etc).

On a different note, from the brief description if the use case you gave on
your first e-mail, I think you can probably get things done without using
a WebKitWebExtension at all. Instead of an extension you could:

- Use WebKitUserContentManager [2] to inject JS scripts that are loaded in
the context of the Web pages loaded (that is: in the WebKitWebProcess).

- Use webkit_web_view_run_javascript() [3] to make calls into functionality
provided by the injected JS code.

- If you need to communicate back from the Web process, you can use a
script message handler (also in WebKitUserContentManager) to allow sending
JS objects to the UI process. Every time one of this is received, it will
trigger the WebKitUserContentManager::script-message-received signal.

While there are certain things which cannot be easily done with JS injected
code, for many things it does a perfectly fine job, and usually with much
less effort.

Cheers,

-Adrián

---
[1] https://stackoverflow.com/questions/6125068/what-does-the-fd-cloexec-fcntl-flag-do
[2] https://webkitgtk.org/reference/webkit2gtk/stable/WebKitUserContentManager.html
[3] https://webkitgtk.org/reference/webkit2gtk/stable/WebKitWebView.html#webkit-web-view-run-javascript

[Michael Catanzaro]

WebKit uses several child processes. A WebKitWebExtension is just a way
to run code in the WebKitWebProcess. Each WebKitWebView corresponds to
one WebKitNetworkProcess and one or more WebKitWebProcess processes.
These will be created regardless of whether or not you use the
WebKitWebExtension class to inject code into the WebKitWebProcess
processes.

On Wed, Dec 5, 2018 at 1:51 PM, Adrian Perez de Castro

Post by Adrian Perez de Castro
Are you setting the close-on-exec flag [1] on the file descriptor? If not,
it may be as well that the file descriptor for the socket is being inherited
by the child processes (WebKitNetworProcess, WebKitWebProcess, etc).

This should not be possible. WebKit creates subprocesses using
g_spawn_async() in ProcessLauncherGLib.cpp. You can review the relevant
code in GLib's gspawn.c. After forking, GLib iterates through all open
file descriptors (except stdin, stdout, and stderr) and sets FD_CLOEXEC
on each one to ensure they get closed when it later calls exec.

Otherwise, if Eclipse failed to set CLOEXEC itself, then the file
descriptors would be leaked in the child process, and you would
probably wind up with this socket in use or not available error. (This
behavior can be disabled using the G_SPAWN_LEAVE_DESCRIPTORS_OPEN flag,
but WebKit does not use this flag.)

So Eric, you say "debugging shows that the web extension still holds
onto the socket/file descriptor." Are you sure the affected file
descriptor is really owned by the WebKitWebProcess? If so, something
very weird is happening inside gspawn.c, because it shouldn't be
possible.

Post by Adrian Perez de Castro
On a different note, from the brief description if the use case you gave on
your first e-mail, I think you can probably get things done without using
a WebKitWebExtension at all.

Yes, if it's possible to avoid the need for a WebKitWebExtension
altogether, that would be ideal.

Michael

[Eric Williams]

Post by Michael Catanzaro
WebKit uses several child processes. A WebKitWebExtension is just a way
to run code in the WebKitWebProcess. Each WebKitWebView corresponds to
one WebKitNetworkProcess and one or more WebKitWebProcess processes.
These will be created regardless of whether or not you use the
WebKitWebExtension class to inject code into the WebKitWebProcess
processes.
On Wed, Dec 5, 2018 at 1:51 PM, Adrian Perez de Castro

Post by Adrian Perez de Castro
Are you setting the close-on-exec flag [1] on the file descriptor? If not,
it may be as well that the file descriptor for the socket is being inherited
by the child processes (WebKitNetworProcess, WebKitWebProcess, etc).

This should not be possible. WebKit creates subprocesses using
g_spawn_async() in ProcessLauncherGLib.cpp. You can review the relevant
code in GLib's gspawn.c. After forking, GLib iterates through all open
file descriptors (except stdin, stdout, and stderr) and sets FD_CLOEXEC
on each one to ensure they get closed when it later calls exec.
Otherwise, if Eclipse failed to set CLOEXEC itself, then the file
descriptors would be leaked in the child process, and you would probably
wind up with this socket in use or not available error. (This behavior
can be disabled using the G_SPAWN_LEAVE_DESCRIPTORS_OPEN flag, but
WebKit does not use this flag.)

This is very useful thank you, I am going to look into the closing of
the port/socket a bit more closely as I am starting to suspect the issue
lies there and not with WebKit.

Post by Michael Catanzaro
So Eric, you say "debugging shows that the web extension still holds
onto the socket/file descriptor." Are you sure the affected file
descriptor is really owned by the WebKitWebProcess? If so, something
very weird is happening inside gspawn.c, because it shouldn't be possible.

This is what was provided to me by the end user, and it's observed that
the bug is reproducible in certain steps, only when a web extension is
created. However this again could be related to the closing of the
port/socket and not a WebKit issue. I'll look into it further.

Post by Michael Catanzaro

Post by Adrian Perez de Castro
On a different note, from the brief description if the use case you gave on
your first e-mail, I think you can probably get things done without using
a WebKitWebExtension at all.

Yes, if it's possible to avoid the need for a WebKitWebExtension
altogether, that would be ideal.

I did not write the WebKit implementation for SWT, but IIRC the reasons
for using an extension are:

1) We have API that allows users to define JS functions which will (when
executed) return values back to SWT at the Java level. We accomplish by
creating an extension, registering a function, and communicating with
the main SWT Java process via DBus.

2) These functions need to be available all the time, meaning they need
to be re-registered whenever a page is refreshed. This means listening
to the "window-object-cleared" signal, and re-registering the JS
functions on page refresh.

3) There is also API which allows an SWT client to execute JS
synchronously which is a bit tricky as webkit_web_view_run_javascript()
is async. My colleague who implemented WebKit in SWT managed to
workaround this by using a web extension and DBus.

Anyway thank you for your help on such a vague kind of question, I'm
going to investigate the matter further with the bug reporter.


Eric