VOOZH about

URL: https://bugs.launchpad.net/mir/+bug/1296544

⇱ Bug #1296544 “helgrind: Lock order violated (potential deadlock)...” : Bugs : Mir

👁 Image

Mir

helgrind: Lock order violated (potential deadlock) in mir::scene::SurfaceStack::emit_change_notification() vs mir::compositor::MultiThreadedCompositor::schedule_compositing()

Bug #1296544 reported by Daniel van Vugt
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mir
Fix Released
Critical
Alberto Aguirre
mir (Ubuntu)
Fix Released
Critical
Unassigned

Bug Description

helgrind: Lock order violated (potential deadlock) in mir::scene::SurfaceStack::emit_change_notification() vs mir::compositor::MultiThreadedCompositor::schedule_compositing()

==19841== ----------------------------------------------------------------
==19841==
==19841== Thread #4: lock order "0xA9FF380 before 0xA9FF108" violated
==19841==
==19841== Observed (incorrect) order is: acquisition of lock at 0xA9FF108
==19841== at 0x4C30795: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==19841== by 0x8F6481: __gthread_mutex_lock(pthread_mutex_t*) (gthr-default.h:748)
==19841== by 0x8F7F67: std::mutex::lock() (mutex:134)
==19841== by 0x9EA695: std::lock_guard<std::mutex>::lock_guard(std::mutex&) (mutex:414)
==19841== by 0x5445AD6: mir::scene::SurfaceStack::emit_change_notification() (surface_stack.cpp:160)
==19841== by 0x54456A4: mir::scene::SurfaceStack::add_surface(std::shared_ptr<mir::scene::Surface> const&, mir::IntWrapper<mir::scene::detail::DepthIdIdTag, int>, mir::input::InputReceptionMode) (surface_stack.cpp:112)
==19841== by 0x54457AF: mir::scene::SurfaceStack::create_surface(mir::IntWrapper<mir::frontend::detail::SessionsSurfaceIdTag, int>, mir::shell::SurfaceCreationParameters const&, std::shared_ptr<mir::frontend::EventSink> const&, std::shared_ptr<mir::shell::SurfaceConfigurator> const&) (surface_stack.cpp:123)
==19841== by 0x5449F0B: mir::scene::SurfaceController::create_surface(mir::IntWrapper<mir::frontend::detail::SessionsSurfaceIdTag, int>, mir::shell::SurfaceCreationParameters const&, std::shared_ptr<mir::frontend::EventSink> const&, std::shared_ptr<mir::shell::SurfaceConfigurator> const&) (surface_controller.cpp:36)
==19841== by 0x544A405: mir::scene::SurfaceSource::create_surface(mir::shell::Session*, mir::shell::SurfaceCreationParameters const&, mir::IntWrapper<mir::frontend::detail::SessionsSurfaceIdTag, int>, std::shared_ptr<mir::frontend::EventSink> const&) (surface_source.cpp:46)
==19841== by 0x5474703: mir::shell::OrganisingSurfaceFactory::create_surface(mir::shell::Session*, mir::shell::SurfaceCreationParameters const&, mir::IntWrapper<mir::frontend::detail::SessionsSurfaceIdTag, int>, std::shared_ptr<mir::frontend::EventSink> const&) (organising_surface_factory.cpp:45)
==19841== by 0x5421602: mir::scene::ApplicationSession::create_surface(mir::shell::SurfaceCreationParameters const&) (application_session.cpp:73)
==19841== by 0x53B1314: mir::frontend::SessionMediator::create_surface(google::protobuf::RpcController*, mir::protobuf::SurfaceParameters const*, mir::protobuf::Surface*, google::protobuf::Closure*) (session_mediator.cpp:169)
==19841==
==19841== followed by a later acquisition of lock at 0xA9FF380
==19841== at 0x4C30795: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==19841== by 0x8F6481: __gthread_mutex_lock(pthread_mutex_t*) (gthr-default.h:748)
==19841== by 0x8F7F67: std::mutex::lock() (mutex:134)
==19841== by 0x8F8328: std::unique_lock<std::mutex>::lock() (mutex:511)
==19841== by 0x8F8110: std::unique_lock<std::mutex>::unique_lock(std::mutex&) (mutex:443)
==19841== by 0x539A2C1: mir::compositor::MultiThreadedCompositor::schedule_compositing() (multi_threaded_compositor.cpp:168)
==19841== by 0x539A4B6: mir::compositor::MultiThreadedCompositor::start()::{lambda()#2}::operator()() const (multi_threaded_compositor.cpp:197)
==19841== by 0x539AC73: std::_Function_handler<void (), mir::compositor::MultiThreadedCompositor::start()::{lambda()#2}>::_M_invoke(std::_Any_data const&) (functional:2071)
==19841== by 0x95EB71: std::function<void ()>::operator()() const (functional:2464)
==19841== by 0x5445AE8: mir::scene::SurfaceStack::emit_change_notification() (surface_stack.cpp:161)
==19841== by 0x54456A4: mir::scene::SurfaceStack::add_surface(std::shared_ptr<mir::scene::Surface> const&, mir::IntWrapper<mir::scene::detail::DepthIdIdTag, int>, mir::input::InputReceptionMode) (surface_stack.cpp:112)
==19841== by 0x54457AF: mir::scene::SurfaceStack::create_surface(mir::IntWrapper<mir::frontend::detail::SessionsSurfaceIdTag, int>, mir::shell::SurfaceCreationParameters const&, std::shared_ptr<mir::frontend::EventSink> const&, std::shared_ptr<mir::shell::SurfaceConfigurator> const&) (surface_stack.cpp:123)
==19841==
==19841== Required order was established by acquisition of lock at 0xA9FF380
==19841== at 0x4C30795: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==19841== by 0x8F6481: __gthread_mutex_lock(pthread_mutex_t*) (gthr-default.h:748)
==19841== by 0x8F7F67: std::mutex::lock() (mutex:134)
==19841== by 0x8F8328: std::unique_lock<std::mutex>::lock() (mutex:511)
==19841== by 0x8F8110: std::unique_lock<std::mutex>::unique_lock(std::mutex&) (mutex:443)
==19841== by 0x539A4EE: mir::compositor::MultiThreadedCompositor::start() (multi_threaded_compositor.cpp:176)
==19841== by 0x5359A1B: mir::DisplayServer::run() (display_server.cpp:208)
==19841== by 0x5350353: mir::run_mir(mir::ServerConfiguration&, std::function<void (mir::DisplayServer&)>) (run_mir.cpp:90)
==19841== by 0x9E06B1: mir_test_framework::TestingProcessManager::launch_server_process(mir_test_framework::TestingServerConfiguration&) (testing_process_manager.cpp:64)
==19841== by 0x9DD292: mir_test_framework::BespokeDisplayServerTestFixture::launch_server_process(mir_test_framework::TestingServerConfiguration&) (display_server_test_fixture.cpp:60)
==19841== by 0x9B0F7B: TestClientInput_clients_receive_key_input_Test::TestBody() (test_client_input.cpp:215)
==19841== by 0xB1BE9B: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (in /home/dan/bzr/mir/toy/build/bin/mir_acceptance_tests)
==19841==
==19841== followed by a later acquisition of lock at 0xA9FF108
==19841== at 0x4C30795: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==19841== by 0x8F6481: __gthread_mutex_lock(pthread_mutex_t*) (gthr-default.h:748)
==19841== by 0x8F7F67: std::mutex::lock() (mutex:134)
==19841== by 0x9EA695: std::lock_guard<std::mutex>::lock_guard(std::mutex&) (mutex:414)
==19841== by 0x54454D8: mir::scene::SurfaceStack::set_change_callback(std::function<void ()> const&) (surface_stack.cpp:96)
==19841== by 0x539A5A8: mir::compositor::MultiThreadedCompositor::start() (multi_threaded_compositor.cpp:198)
==19841== by 0x5359A1B: mir::DisplayServer::run() (display_server.cpp:208)
==19841== by 0x5350353: mir::run_mir(mir::ServerConfiguration&, std::function<void (mir::DisplayServer&)>) (run_mir.cpp:90)
==19841== by 0x9E06B1: mir_test_framework::TestingProcessManager::launch_server_process(mir_test_framework::TestingServerConfiguration&) (testing_process_manager.cpp:64)
==19841== by 0x9DD292: mir_test_framework::BespokeDisplayServerTestFixture::launch_server_process(mir_test_framework::TestingServerConfiguration&) (display_server_test_fixture.cpp:60)
==19841== by 0x9B0F7B: TestClientInput_clients_receive_key_input_Test::TestBody() (test_client_input.cpp:215)
==19841== by 0xB1BE9B: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (in /home/dan/bzr/mir/toy/build/bin/mir_acceptance_tests)
==19841==

Tags: helgrind

Related branches

lp:~albaguirre/mir/fix-1296544
Alan Griffiths: Approve
PS Jenkins bot (community): Approve (continuous-integration)
Robert Carr (community): Approve
Alexandros Frantzis (community): Approve
Chris Halse Rogers: Approve
Diff: 338 lines (+170/-48)
3 files modified
src/server/compositor/multi_threaded_compositor.cpp (+97/-42)
src/server/compositor/multi_threaded_compositor.h (+13/-2)
tests/unit-tests/compositor/test_multi_threaded_compositor.cpp (+60/-4)
api
Revision history for this message 👁 Image
Daniel van Vugt (vanvugt) wrote : #1

Produced by running:
valgrind --tool=helgrind --log-file=out.txt bin/mir_acceptance_tests --gtest_filter="TestClientInput.clients_receive_key_input"

Changed in mir:
importance: High → Critical
Changed in mir:
milestone: 0.1.8 → 0.1.9
Changed in mir:
assignee: nobody → Alberto Aguirre (albaguirre)
status: Triaged → In Progress
Revision history for this message 👁 Image
PS Jenkins bot (ps-jenkins) wrote : #2

Fix committed into lp:mir/devel at revision None, scheduled for release in mir, milestone Unknown

Changed in mir:
status: In Progress → Fix Committed
Changed in mir:
status: Fix Committed → Fix Released
Changed in mir (Ubuntu):
importance: Undecided → Critical
status: New → Triaged
Revision history for this message 👁 Image
Launchpad Janitor (janitor) wrote : #3

This bug was fixed in the package mir - 0.1.9+14.10.20140430.1-0ubuntu1

---------------
mir (0.1.9+14.10.20140430.1-0ubuntu1) utopic; urgency=medium

  [ Daniel van Vugt ]
  * New upstream release 0.1.9 (https://launchpad.net/mir/+milestone/0.1.9)
    - mirclient ABI unchanged, still at 7. Clients do not need rebuilding.
    - mirserver ABI bumped to 19. Shells need rebuilding.
    - More libmirserver class changes and reorganization, including;
      . Moving things from shell:: to scene::
      . Rewriting/refactoring surface factories.
    - Added an id() to Renderable.
    - Scene/Renderer interfaces:
      . Scene is no longer responsible for its own iteration (no for_each
        any more). Instead you should iterate over the list returned by
        Scene::generate_renderable_list().
    - Bugs fixed:
      . Stale socket issue. (LP: #1285215)
      . Qt render gets blocked on EGLSwapBuffers. (LP: #1292306)
      . Lock order violated found in helgrind (potential deadlock).
        (LP: #1296544)
      . [regression] SwitchingBundle in framedropping mode can hang.
        (LP: #1306464)
      . [DPMS] Display backlight turns back on almost immediately after
        being turned off. (LP: #1231857)
      . Wrong frame is seen on wake up/resume/unlock. (LP: #1233564)
      . Nested platform is not testable (LP: #1299101)
      . [regression] mir_demo_server_shell crashes on display resume.
        (LP: #1308941)
      . Multi-threaded composition is actually mostly serialized by
        SurfaceStack::guard. (LP: #1234018)
      . Mirscreencast slows down compositing and makes it very jerky.
        (LP: #1280938)
      . Mirscreencast can cause clients to render faster than the screen
        refresh rate. (LP: #1294361)
      . Screen turns on when a new session/surface appears. (LP: #1297876)
      . mir-doc package is >56MB in size, expands to >100MB of files.
        (LP: #1304998)
      . [regression] Clang: 'mir::test::doubles::MockSurface::visible'
        hides overloaded virtual function [-Woverloaded-virtual].
        (LP: #1301135)
      . [regression] GLRenderer* unit tests have recently become noisy.
        (LP: #1308905)
      . FocusController::set_focus_to() no longer seems to raise a session
        to the top. (LP: #1302689)

  [ Ubuntu daily release ]
  * New rebuild forced
 -- Ubuntu daily release <email address hidden> Wed, 30 Apr 2014 13:26:58 +0000

Changed in mir (Ubuntu):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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

👁 Launchpad
 •  Take the tour  •  Read the guide  
© 2004 Canonical Ltd.  •  Terms of use  •  Data privacy  •  Contact Launchpad Support  •  Blog  •  Careers  •  System status  •  c38ea58 (Get the code!)