Mir

Setting a surface keymap crashes in xkbcommon: xkb_keymap_new_from_names()

Bug #1530946 reported by Lukáš Tinkl
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Canonical Pocket Desktop
Triaged
High
Stephen M. Webb
Canonical System Image
Fix Committed
High
Stephen M. Webb
Canonical System Image 9.5 "OTA.5"
Mir
Fix Released
High
Andreas Pokorny
Mir 0.19.0
The Avila project
Fix Committed
High
Unassigned
The Avila project ww04-2016
mir (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

Trying to set a keymap on a Mir surface crashes somewhere between Mir and xkbcommon with this backtrace:

http://paste.ubuntu.com/14399228/

Code from: https://code.launchpad.net/~lukas-kde/qtmir/kbdLayout and https://code.launchpad.net/~lukas-kde/unity8/kbdLayout

The xkb_names structure (four char*) is sent over the wire as four addresses. Our tests do not fail because they use the in process client..

See original description

Related branches

lp://staging/~andreas-pokorny/mir/fix-1530946
PS Jenkins bot (community): Approve (continuous-integration)
Mir CI Bot: Approve (continuous-integration)
Alan Griffiths: Approve
Chris Halse Rogers: Approve
Alberto Aguirre (community): Approve
Diff: 1416 lines (+544/-220)
33 files modified
include/client/mir/events/event_builders.h (+2/-1)
include/client/mir_toolkit/events/keymap_event.h (+25/-1)
include/server/mir/scene/null_surface_observer.h (+1/-1)
include/server/mir/scene/surface.h (+2/-1)
include/server/mir/scene/surface_observer.h (+4/-2)
include/test/mir/test/doubles/stub_surface.h (+2/-1)
include/test/mir/test/event_matchers.h (+2/-15)
src/client/event.cpp (+23/-6)
src/client/event_printer.cpp (+1/-8)
src/client/events/CMakeLists.txt (+2/-0)
src/client/events/event_builders.cpp (+41/-46)
src/client/events/make_empty_event.cpp (+42/-0)
src/client/events/make_empty_event.h (+32/-0)
src/client/events/serialization.cpp (+111/-0)
src/client/input/xkb_mapper.cpp (+70/-45)
src/client/mir_surface.cpp (+8/-4)
src/client/rpc/mir_protobuf_rpc_channel.cpp (+45/-46)
src/client/symbols.map (+4/-0)
src/include/client/mir/events/serialization.h (+35/-0)
src/include/common/mir/events/event_private.h (+3/-1)
src/include/common/mir/input/xkb_mapper.h (+22/-7)
src/include/server/mir/scene/surface_event_source.h (+2/-1)
src/include/server/mir/scene/surface_observers.h (+2/-1)
src/server/frontend/event_sender.cpp (+4/-3)
src/server/scene/basic_surface.cpp (+7/-5)
src/server/scene/basic_surface.h (+2/-1)
src/server/scene/legacy_surface_change_notification.cpp (+2/-1)
src/server/scene/legacy_surface_change_notification.h (+2/-1)
src/server/scene/null_surface_observer.cpp (+5/-1)
src/server/scene/surface_event_source.cpp (+6/-2)
tests/acceptance-tests/test_client_input.cpp (+31/-17)
tests/include/mir/test/doubles/stub_scene_surface.h (+2/-1)
tests/mir_test_framework/stub_surface.cpp (+2/-1)
Daniel van Vugt (vanvugt)
summary: - Setting a surface keymap crashes in xkbcommon
+ Setting a surface keymap crashes in xkbcommon:
+ xkb_keymap_new_from_names()
Changed in mir:
importance: Undecided → High
Alberto Aguirre (albaguirre)
Changed in mir:
status: New → Invalid
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

Have you tried using empty strings instead of null for unused fields?

Changed in mir:
status: Invalid → Confirmed
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Yup, similar crash

Revision history for this message
Alberto Aguirre (albaguirre) wrote :

so I hacked TestClientInput.clients_receive_keymap_change_events (tests/acceptance_tests/test_client_input.cpp) to do:

 xkb_rule_names names;
  names.rules = nullptr;
  names.model = nullptr;
  names.layout = "us";
  names.variant = nullptr;
  names.options = nullptr;

EXPECT_CALL(first_client, handle_keymap(_))
        .Times(1)
        .WillOnce(mt::WakeUp(&first_client.all_events_received));

server.the_shell()->focused_surface()->set_keymap(names);
first_client.all_events_received.wait_for_at_most_seconds(2);

No crash.

Can you examine xkb_rule_names and list the contents?

Changed in mir:
status: Confirmed → Incomplete
assignee: nobody → Alberto Aguirre (albaguirre)
Andreas Pokorny (andreas-pokorny)
Changed in mir:
assignee: Alberto Aguirre (albaguirre) → Andreas Pokorny (andreas-pokorny)
status: Incomplete → Confirmed
description: updated
Michał Sawicz (saviq)
Changed in canonical-devices-system-image:
assignee: nobody → Stephen M. Webb (bregma)
milestone: none → ww04-2016
importance: Undecided → Critical
status: New → Triaged
Changed in canonical-pocket-desktop:
status: New → Triaged
importance: Undecided → High
Changed in canonical-devices-system-image:
importance: Critical → High
Changed in canonical-pocket-desktop:
assignee: nobody → Stephen M. Webb (bregma)
Brandon Schaefer (brandontschaefer)
Changed in mir:
milestone: none → 0.19.0
Daniel van Vugt (vanvugt)
Changed in mir:
status: Confirmed → In Progress
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:mir at revision 3266, scheduled for release in mir, milestone 0.20.0

Changed in mir:
status: In Progress → Fix Committed
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

mir (0.19.0+16.04.20160128-0ubuntu1) xenial; urgency=medium

Changed in mir:
status: Fix Committed → Fix Released
Changed in mir (Ubuntu):
status: New → Fix Released
Jean-Baptiste Lallement (jibel)
Changed in canonical-devices-system-image:
status: Triaged → Fix Committed
John McAleely (john.mcaleely)
Changed in avila:
status: New → Fix Committed
milestone: none → ww04-2016
importance: Undecided → High
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.