Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Firefox-wayland crash when selection is available or clipboard has contents #1041

Closed
martinetd opened this issue Jun 5, 2018 · 8 comments · Fixed by #1384
Closed

Firefox-wayland crash when selection is available or clipboard has contents #1041

martinetd opened this issue Jun 5, 2018 · 8 comments · Fixed by #1384
Labels

Comments

@martinetd
Copy link
Member

I think I need help with this.
Here's the logs on our (rootston) side, because we don't have the last .selection() event on FF side as it crashes:

[1404702.976]  -> gtk_primary_selection_device@25.data_offer(new id gtk_primary_selection_offer@4278190080)
[1404703.009]  -> gtk_primary_selection_device@15.data_offer(new id gtk_primary_selection_offer@4278190080)
[1404703.039]  -> gtk_primary_selection_offer@4278190080.offer("text/plain")
[1404703.064]  -> gtk_primary_selection_offer@4278190080.offer("text/plain;charset=utf-8")
[1404703.089]  -> gtk_primary_selection_device@25.selection(gtk_primary_selection_offer@4278190080)
[1404703.113]  -> gtk_primary_selection_device@15.selection(gtk_primary_selection_offer@4278190080)

Where both selection devices have been created on different wl_seat (logs on ff side this time)

[191882.862]  -> gtk_primary_selection_device_manager@10.get_device(new id gtk_primary_selection_device@15, wl_seat@13)
[192201.245]  -> gtk_primary_selection_device_manager@22.get_device(new id gtk_primary_selection_device@25, wl_seat@23)

tbh even if it was a single seat it'd probably crash the app just the same to instanciate multiple device managers/selection devices and use them, so not sure we should focus on that part..

So apparently it's not OK to offer up the same selection to multiple devices.
Either we need to instanciate a different gtk_primary_selection_offer and offer one each, or we need to pick our poison and give it to one.
weston doesn't seem to have a complete selection implementation?
gnome-shell only gives it to one, presumably the latest device manager instanciated.

What shall we do?

@Ongy
Copy link

Ongy commented Jun 5, 2018

I'd say if a client created to devices it signaled interest twice and this should be a bug on the firefox project and GNOME, not here.

@martinetd
Copy link
Member Author

What strikes me as possibly wrong is that we're using the same gtk_primary_selection_offer (same id) while the call is of type=new_id so means allocation.
I think if we send it twice, we need to allocate (and fill!) one per selection device.

@martinetd
Copy link
Member Author

Hm now I'm thinking a bit more we do the same with the selection device_manager.get_device that always return the same id for a given seat, so that might be ok. I don't know.

@Ongy
Copy link

Ongy commented Jun 5, 2018

Oh, yea. If it's new_id it probably allocates a new proxy and could lead to double-free like problems.
Though I thought they are client allocated since the ids are in the high areas.
I should take a look at the protocol I guess

@Ongy
Copy link

Ongy commented Jun 5, 2018

Ok, yea. new_id stuff should probably have unique ids.

new_device is a request, so it should be client generated id, while data_offer is server provided. So it probably creates to wl_proxy on the client side and that somehow crashes things.

Then I correct myself and say we should probably create two offers instead.

@emersion emersion added the bug label Jun 5, 2018
@aereaux
Copy link
Contributor

aereaux commented Jun 21, 2018

Here is a hacky workaround for this issue. I plan on looking into it more and trying to get a better solution at some point.

@Ranguvar
Copy link

Can this be closed? No crashes in Firefox since early 65 hg.

@martinetd
Copy link
Member Author

No, we're still not handling this properly by sending the same object multiple times when we should allocate different ones, this needs to be fixed.
I've started something but left it a bit unloved as I don't find it very nice, and it'll need a lot of churn to extend to data_offer that needs the same fix, and it's been a bit buggy/I never took time to debug it. . . emersion was nice enough to say he'd finish it up eventually but if you're like to help feel free to look at my PR (#1113) and do something similar for data_offer; I honestly don't think I would have time to finish this in the short term sorry :(

@emersion emersion changed the title Firefox-wayland crash when selection is available Firefox-wayland < 65 crash when selection is available Nov 13, 2018
emersion added a commit to emersion/wlroots that referenced this issue Nov 21, 2018
When a client was creating multiple data devices for the same seat, we were
only creating one resource. This is a protocol error.

Instead, create one offer per data device.

This commit also makes offers inert when their source is destroyed.

Fixes part of swaywm#1041
Supersedes swaywm#1113
emersion added a commit to emersion/wlroots that referenced this issue Nov 21, 2018
When a client was creating multiple data devices for the same seat, we were
only creating one resource. This is a protocol error.

Instead, create one offer per data device.

This commit also makes offers inert when their source is destroyed.

Fixes part of swaywm#1041
Supersedes swaywm#1113
@emersion emersion changed the title Firefox-wayland < 65 crash when selection is available Firefox-wayland crash when selection is available or clipboard has contents Nov 21, 2018
emersion added a commit to emersion/wlroots that referenced this issue Nov 21, 2018
This commit makes it possible for a single client to have multiple data devices
for the same seat. This fixes issues with Firefox.

This mainly removes wlr_data_source.offer. We make sure we create one data
offer per device.

Fixes the second half of swaywm#1041
emersion added a commit to emersion/wlroots that referenced this issue Nov 21, 2018
This commit makes it possible for a single client to have multiple data devices
for the same seat. This fixes issues with Firefox.

This mainly removes wlr_data_source.offer. We make sure we create one data
offer per device. We now make the offer inert when the source is destroyed.

Fixes the second half of swaywm#1041
emersion added a commit to emersion/wlroots that referenced this issue Nov 26, 2018
This commit makes it possible for a single client to have multiple data devices
for the same seat. This fixes issues with Firefox.

This mainly removes wlr_data_source.offer. We make sure we create one data
offer per device. We now make the offer inert when the source is destroyed.

Fixes the second half of swaywm#1041
emersion added a commit to emersion/wlroots that referenced this issue Nov 26, 2018
This commit makes it possible for a single client to have multiple data devices
for the same seat. This fixes issues with Firefox.

This mainly removes wlr_data_source.offer. We make sure we create one data
offer per device. We now make the offer inert when the source is destroyed.

Fixes the second half of swaywm#1041
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

5 participants