Code review comment for lp://qastaging/~bilalakhtar/unity/sc-integration-phase2

Hi,

Upon further investigation, the crash is inside the software center launcher icon. I get a segfault about 50% of the time when running the autopilot test. Here's the stack trace:

#0 0x00007fffe4f02ca2 in unity::launcher::SoftwareCenterLauncherIcon::Animate (this=0x0, launcher=..., icon_x=100, icon_y=100, icon_size=32)
    at /home/thomi/code/canonical/unity/sc-integration-phase2/plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp:92
#1 0x00007fffe509ecd7 in unity::launcher::Controller::Impl::OnLauncherAddRequestSpecial (this=0x903c90, path=..., before=..., aptdaemon_trans_id=..., icon_path=...,
    icon_x=100, icon_y=100, icon_size=32) at /home/thomi/code/canonical/unity/sc-integration-phase2/plugins/unityshell/src/LauncherController.cpp:406
#2 0x00007fffe50aa989 in sigc::bound_mem_functor7<void, unity::launcher::Controller::Impl, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int>::operator() (this=0xd24498, _A_a1=..., _A_a2=..., _A_a3=..., _A_a4=..., _A_a5=@0x7fffffffe0f0: 100,
    _A_a6=@0x7fffffffe0f4: 100, _A_a7=@0x7fffffffe0f8: 32) at /usr/include/sigc++-2.0/sigc++/functors/mem_fun.h:2277
#3 0x00007fffe50a9d5f in sigc::adaptor_functor<sigc::bound_mem_functor7<void, unity::launcher::Controller::Impl, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int> >::operator()<std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon> const&, std::string const&, std::string const&, int const&, int const&, int const&> (this=0xd24490, _A_arg1=..., _A_arg2=..., _A_arg3=..., _A_arg4=...,
    _A_arg5=@0x7fffffffe0f0: 100, _A_arg6=@0x7fffffffe0f4: 100, _A_arg7=@0x7fffffffe0f8: 32) at /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:213
#4 0x00007fffe50a89b2 in sigc::internal::slot_call7<sigc::bound_mem_functor7<void, unity::launcher::Controller::Impl, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int>, void, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int>::call_it (rep=0xd24460, a_1=..., a_2=..., a_3=..., a_4=..., a_5=@0x7fffffffe0f0: 100, a_6=@0x7fffffffe0f4: 100,
    a_7=@0x7fffffffe0f8: 32) at /usr/include/sigc++-2.0/sigc++/functors/slot.h:383
#5 0x00007fffe508b1e9 in sigc::internal::signal_emit7<void, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int, sigc::nil>::emit (impl=0xd24410, _A_a1=..., _A_a2=..., _A_a3=..., _A_a4=..., _A_a5=@0x7fffffffe0f0: 100, _A_a6=@0x7fffffffe0f4: 100,
    _A_a7=@0x7fffffffe0f8: 32) at /usr/include/sigc++-2.0/sigc++/signal.h:2567
#6 0x00007fffe508995d in sigc::signal7<void, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int, sigc::nil>::emit (this=0xca1690, _A_a1=..., _A_a2=..., _A_a3=..., _A_a4=..., _A_a5=@0x7fffffffe0f0: 100, _A_a6=@0x7fffffffe0f4: 100, _A_a7=@0x7fffffffe0f8: 32)
    at /usr/include/sigc++-2.0/sigc++/signal.h:3471
#7 0x00007fffe508737e in unity::launcher::Launcher::handle_dbus_method_call (connection=0x7fffd4006810, sender=0x7fffcc01d0f0 ":1.420",
    object_path=0x7fffcc011c20 "/com/canonical/Unity/Launcher", interface_name=0x7fffcc024a20 "com.canonical.Unity.Launcher",
    method_name=0x7fffcc027af0 "AddLauncherItemFromPosition", parameters=0x7fffcc02e0a0, invocation=0x2688ea0, user_data=0xca1240)
    at /home/thomi/code/canonical/unity/sc-integration-phase2/plugins/unityshell/src/Launcher.cpp:2856
#8 0x00007ffff1bd6b5d in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#9 0x00007ffff5bacdda in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x00007ffff5bad1a0 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#11 0x00007ffff5bad59a in g_main_loop_run () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#12 0x00000000004029ae in main ()

This seems to be caused by the fact that in /plugins/unityshell/src/LauncherController.cpp line 406 you do this:

static_cast<SoftwareCenterLauncherIcon*>(result.GetPointer())->Animate(launcher_, icon_x, icon_y, icon_size);

without checking that the result from "result.GetPOinter()" is valid. The CreateSCLauncherIcon method that creates the icon you're calling looks like this - I've added some notes to show what's wrong:

706 Controller::Impl::CreateSCLauncherIcon(std::string const& file_path,
707 std::string const& aptdaemon_trans_id,
708 std::string const& icon_path)
709 {
710 BamfApplication* app;
711 AbstractLauncherIcon::Ptr result;

Here, 'result' is a null ptr.

712
713 app = bamf_matcher_get_application_for_desktop_file(matcher_, file_path.c_str(), true);
714 if (!BAMF_IS_APPLICATION(app))
715 return result;

Here, you're returning a null ptr.

716
717 if (g_object_get_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen")))
718 {
719 bamf_view_set_sticky(BAMF_VIEW(app), true);
720 return result;

... and same here too.

721 }
722
723 g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"), GINT_TO_POINTER(1));
724
725 bamf_view_set_sticky(BAMF_VIEW(app), true);
726 AbstractLauncherIcon::Ptr icon(new SoftwareCenterLauncherIcon(app, aptdaemon_trans_id, icon_path));

Here you create the icon...

727 icon->SetSortPriority(sort_priority_++);
728
729 result = icon;

...and finally assign it to 'result'.

730 return result;
731 }

Only if all those if statements pass will this do what you want. Inf act, what's happening is that the second time I run the AP test, this if statement:

717 if (g_object_get_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen")))
718 {
719 bamf_view_set_sticky(BAMF_VIEW(app), true);
720 return result;
721 }

...evaluates to true, and you return null.

I am still working on why the autopilot test gives you an error on cleanup, but this needs to be fixed before I can run the autopilot tests repeatedly. Please ping me on IRC if you have any questions.

Cheers,