Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segfault when cursor moves after monitor plugged in #1009

Closed
AlanGriffiths opened this issue Sep 24, 2019 · 2 comments · Fixed by #1010 or #1016
Closed

Segfault when cursor moves after monitor plugged in #1009

AlanGriffiths opened this issue Sep 24, 2019 · 2 comments · Fixed by #1010 or #1016
Assignees

Comments

@AlanGriffiths
Copy link
Contributor

AlanGriffiths commented Sep 24, 2019

(There may be simpler, but this works for me.)

Run up miral-app with two screens L and R. Terminal appears on R.

Use mouse to move the terminal window to L, leaving mouse over titlebar.

Unplug L, wait for screen to update (terminal appears on R as it's now at 0,0).

Replug L, wait for screen to update (terminal appears on L as it's now at 0,0).

Move cursor off window.

Expect: system continues to work.
Actual: segfault at src/platforms/mesa/server/kms/cursor.cpp:296

@AlanGriffiths AlanGriffiths self-assigned this Sep 24, 2019
@AlanGriffiths AlanGriffiths changed the title Segfault when monitor plugged in Segfault when cursor moves after monitor plugged in Sep 24, 2019
@bors bors bot closed this as completed in 391da11 Sep 24, 2019
@bors bors bot closed this as completed in #1010 Sep 24, 2019
@AlanGriffiths AlanGriffiths reopened this Sep 30, 2019
@AlanGriffiths
Copy link
Contributor Author

AlanGriffiths commented Sep 30, 2019

Some more detail on the scenario that provokes this:

Qt (at least in this scenario) uses a single surface and updates the buffer. The error is occurring when a bufferstream has been used to draw the pointer and WlPointer::set_cursor() is called immediately prior to the new (e.g. sizing) image being sent. (I'm still not clear why the previous buffer is invalid, but the existence of "log_warning("Attempt to read from WlShmBuffer after the wl_buffer has been destroyed");" suggests that this can indeed happen.)

@AlanGriffiths
Copy link
Contributor Author

OK, the problem is that CursorImageFromBuffer assumes that PixelSource::read() will execute the do_with_pixels block. But this is not true of the WlShmBuffer::read() implementation.

bors bot added a commit that referenced this issue Sep 30, 2019
1016: PixelSource::read() should execute the supplied block r=RAOF a=AlanGriffiths

PixelSource::read() should execute the supplied block. (Fixes: #1009)

The WlShmBuffer::read() implementation failed to execute the do_with_pixels block when the underlying wl_buffer has been destroyed.

As the most recent buffer remains in the bufferstream until another buffer is submitted client code (e.g. CursorImageFromBuffer) cannot know that the PixelSource is no longer supported by an underlying wl_buffer.

I've removed the log that this is a logic_error as I don't see how the calling code can know.

Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
@bors bors bot closed this as completed in 1420de7 Oct 1, 2019
@bors bors bot closed this as completed in #1016 Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant