Crash when dragging video onto running mpv on wayland

[DorianRudolph]

Important Information

Provide following Information:

• mpv version: 0.32.0 (also reproducible on master build)
• Linux Distribution and Version: Arch Linux (5.7.10-arch1-1)
• Source of the mpv binary: Arch Repo
• Window Manager and version: Sway 1:1.5-1
• GPU driver and version: amdgpu

Reproduction steps

Drag a video file from another application (I used Dolphin) onto a playing mpv window.

Note that mpv does not crash if I open both videos (mpv a.mkv b.mkv) and then use the arrow buttons to switch the video.

Expected behavior

mpv plays the video.

Actual behavior

mpv crashes with

wl_data_offer@4278190080: error 0: Offer is not drag-and-drop
[vo/gpu/wayland] Error occurred on the display fd, closing

Log file

mpv.log

[Dudemanguy]

Works for me (same version of sway and also using amdgpu). Maybe there's something more specific you have to to do trigger it.

[LaserEyess]

Getting this as well

https://0x0.st/ixc9.txt

[   5.408][v][vo/gpu/wayland] Given DND offer with mime type text/uri-list
[   5.408][v][vo/gpu/wayland] DND action is DND_APPEND
[   5.408][v][vo/gpu/wayland] Accepting DND offer with mime type text/uri-list
[   5.412][v][vo/gpu/wayland] DND action is DND_REPLACE
[   5.710][v][vo/gpu/wayland] Receiving DND offer with mime text/uri-list
[   5.713][v][vo/gpu/wayland] Read 38 bytes from the DND fd
[   5.713][d][cplayer] Run command: loadfile, flags=65, args=[url="file:///home/laser/Videos/fuckyou.webm", flags="replace", o>
[   5.713][v][cplayer] EOF code: 3  

[Dudemanguy]

Weird. Everything I've tried so far works. https://0x0.st/ixc4.log

[  12.529][v][vo/gpu/wayland] Given DND offer with mime type text/uri-list
[  12.529][v][vo/gpu/wayland] DND action is DND_APPEND
[  12.529][v][vo/gpu/wayland] Accepting DND offer with mime type text/uri-list
[  12.529][v][vo/gpu/wayland] DND action is DND_REPLACE
[  12.797][v][vo/gpu/wayland] Receiving DND offer with mime text/uri-list
[  12.831][v][vo/gpu/wayland] Read 143 bytes from the DND fd
[  12.831][d][cplayer] Run command: loadfile, flags=65, args=[url="file:///media/sdb/Anime/[Amurrica]%20Rose%20of%20Versailles/[Amurrica]%20Rose%20of%20Versailles%20-%2001%20[BD-1080p-10bit-FLAC][F7839EB9].mkv", flags="replace", options=""]
[  12.831][v][cplayer] EOF code: 3  
[  12.831][d][decode_wrapper/ad] Uninit decoder.
[  12.832][d][decode_wrapper/vd] Uninit decoder.
[  12.833][d][cplayer] Terminating demuxers...
[  12.833][d][cplayer] Done terminating demuxers.
[  12.833][v][cplayer] finished playback, success (reason 2)
[  12.833][i][cplayer] 
[  12.833][d][global] config path: 'watch_later' -> '-'
[  12.833][v][cplayer] Running hook: ytdl_hook/on_load
[  12.833][v][ytdl_hook] ytdl:// hook 
[  12.834][v][ytdl_hook] not a ytdl:// url 
[  12.834][v][bdmv/bluray] Opening /media/sdb/Anime/[Amurrica] Rose of Versailles/[Amurrica] Rose of Versailles - 01 [BD-1080p-10bit-FLAC][F7839EB9].mkv
[  12.834][v][file] Opening /media/sdb/Anime/[Amurrica] Rose of Versailles/[Amurrica] Rose of Versailles - 01 [BD-1080p-10bit-FLAC][F7839EB9].mkv
[  12.834][d][file] resize stream to 131072 bytes, drop 0 bytes
[  12.834][d][file] Stream opened successfully.

Anyone feel like getting a backtrace on this?

[Dudemanguy]

Regarding some IRC discussion yesterday, the backtrace didn't have anything strange in it so it wasn't too helpful. Can either of you reproduce this in weston?

[DorianRudolph]

I will try it on weston. I just debugged this on my machine and made an interesting observation. The error occurs in https://github.com/swaywm/wlroots/blob/ca45f4490ccce64bf7aa0985951319646b55d258/types/data_device/wlr_data_offer.c#L133 which means that offer->type != WLR_DATA_OFFER_DRAG, where WLR_DATA_OFFER_DRAG is defined as 1 (this corresponds to byte 16 in struct wl_data_offer). If I set a breakpoint at https://github.com/mpv-player/mpv/blob/6e3d4aa94b170fa6de1bdda32d799cef8648167d/video/out/wayland_common.c#L1461 and then continue execution, the video plays fine.

So I added the following print statement:

printf("####   %d\n", ((char*)wl->dnd_offer)[16]);
wl_data_offer_finish(wl->dnd_offer);

And if I just run mpv without a breakpoint, it will print 0. If I place a breakpoint at the wl_data_offer_finish line, it still prints 0, but if I type ((char*)wl->dnd_offer)[16] into gdb, I get sometimes 1 but mostly 0. It still works though, so it seems the compositor still receives the 1.

This looks like a race condition to me. Looking at gdb, there is certainly a lot of threading going on.

[DorianRudolph]

The issue does not occur under weston. But weston also does not check the offer->type (https://github.com/wayland-project/weston/blob/556afd14826733e240a55798760dab13b7c0a220/libweston/data-device.c#L217).

[ghost]

Why does it close the display just because drag and drop failed? Who do you blame for that? At worst, D6D should just not work.

[Dudemanguy]

The compositor sends an error over the display fd which gets picked up in wait events which then closes the display. That could certainly be reworked/changed to be a non-fatal error. I'm not really sure there's a reason to make it a fatal one.

As for the bug itself, wayland is supposed to inherently asynchronous so it really shouldn't race. At least not inherently in the protocol or anything like that. Well technically the part of wait events that reads the display fd is wrong so maybe fixing that will help.

[DorianRudolph]

Some additional observations: I just rebooted my PC and then the error went away for some time. I was able to drag videos into mpv many times successfully. At some point the error occurred again and continued to occur for each drag. Now I exited from sway and logged back in and I can no longer reproduce the error. Maybe it will come back again later.

[DorianRudolph]

I think I figured out where the bug comes from. During a single drag operation, mpv receives two data offers in data_device_handle_data_offer, but only the first one is the drag. The drag offer is received when the dragged item is hovered over the window. Once I release the mouse button, a select offer is sent (no idea why) which then replaces the existing offer in wl->dnd_offer. Then mpv calls wl_data_offer_finish with a selection offer after destroying the drag offer.

Do demonstrate this, I added two print statements:

fprintf(stderr, "\n###A %d %p  \n\n", ((char*)wl->dnd_offer)[16], wl->dnd_offer);
wl_data_offer_finish(wl->dnd_offer);

fprintf(stderr, "\n###R %d %p   \n\n", ((char*)wl->dnd_offer)[16], wl->dnd_offer);
wl_data_offer_add_listener(id, &data_offer_listener, wl);

When triggering the bug, the output looks as follows. The first line is printed when starting mpv. The 0/1 is whether it is a select/drop offer.

###R 0 0x7f65387b9e90
###R 1 0x7f65389a8560
###R 0 0x7f65389ab820
###A 0 0x7f65389ab820

It would be correct to call wl_data_offer_finish(0x7f65389a8560) because that is the drop event, but instead the wl->dnd_offer_entry was overriden with 0x7f65389ab820 and 0x7f65389a8560 destroyed.

Here is the a full log with WAYLAND_DEBUG=1: data_offer_crash.txt

When debugging, almost the same thing happens. data_offer_debug.txt

I'm not quite sure why it does not crash when debugging. The only difference I can see in the outputs is that the loadfile command is issued before the ###A (without debugging, they occur the other way around. Since it outputs ###A 0, the error should still occur, but maybe the load somehow makes it swallow the error.

[Dudemanguy]

Yeah actually I was thinking that wl_data_offer_finish should be handled in data_device_handle_drop whether it fixes this bug or not. Does this incredibly naive patch change anything?

diff --git a/video/out/wayland_common.c b/video/out/wayland_common.c
index efb1a48ef5..26cdac814e 100644
--- a/video/out/wayland_common.c
+++ b/video/out/wayland_common.c
@@ -763,6 +763,7 @@ static void data_device_handle_drop(void *data, struct wl_data_device *wl_ddev)
     close(pipefd[1]);

     wl->dnd_fd = pipefd[0];
+    wl_data_offer_finish(wl->dnd_offer);
 }

 static void data_device_handle_selection(void *data, struct wl_data_device *wl_ddev,
@@ -1458,7 +1459,6 @@ static void check_dnd_fd(struct vo_wayland_state *wl)
                                 file_list, wl->dnd_action);
         talloc_free(buffer);
 end:
-        wl_data_offer_finish(wl->dnd_offer);
         talloc_free(wl->dnd_mime_type);
         wl->dnd_mime_type = NULL;
         wl->dnd_mime_score = 0;

Not sure yet if the dnd handling needs just some small fixes or a rewrite.

[DorianRudolph]

Yes that seems to fix the bug.

[Dudemanguy]

Great, I'll mull over check_dnd_fd a bit more and decide whether it's doing other things it shouldn't be doing.

[LaserEyess]

This patch works for me as well

[Max-Enrik]

@Dudemanguy thank you for mentioned this post. I'd like to say I use mpv on PC. Its a bit hard to understand for me to what I have to change and where? Can you help me?

[LaserEyess]

You need to build mpv from source, or patch your package manager's mpv package with https://github.com/mpv-player/mpv/commit/700f4ef5fad353800fa866b059663bc1dd58d3b7 cherry-picked. See https://github.com/mpv-player/mpv#compilation

[Max-Enrik]

I am worrying that I won't make this for me. I copied 'wayland_common.c' file and paste in mpv/video/out/wayland_common.c But nothing changed. I am not coder.

[LaserEyess]

I can't help you more than that, sorry. This is also not the place to ask, try #mpv on freenode.

[Max-Enrik]

Thank you for your time. I appreciate that.