serious bug that might cause memory leaks
Bug #1183507 reported by
Eleni Maria Stea
This bug affects 1 person
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Mir |
Fix Released
|
Medium
|
Eleni Maria Stea |
Bug Description
I found a serious bug in several mir classes which was that classes with virtual functions did not have a virtual destructor as they should.
Whenever such a class is used polymorphically with an instance of a subclass being used through a pointer of "base class" type:
if an attempt is made to delete it through that pointer, only the destructor of the base class will be called resulting in a memory leak since the decision of which destructor to call is taken at compile time for non-virtual functions.
Related branches
lp:~hikiko/mir/mir.dest-tmp
- Daniel van Vugt: Approve
- Alan Griffiths: Approve
- PS Jenkins bot (community): Approve (continuous-integration)
-
Diff: 429 lines (+47/-13)29 files modified3rd_party/cucumber-cpp/include/cucumber-cpp/internal/hook/HookRegistrar.hpp (+3/-0)
3rd_party/cucumber-cpp/include/cucumber-cpp/internal/hook/Tag.hpp (+1/-0)
3rd_party/cucumber-cpp/include/cucumber-cpp/internal/step/StepManager.hpp (+3/-0)
CMakeLists.txt (+1/-1)
include/server/mir/compositor/buffer_allocation_strategy.h (+1/-1)
include/server/mir/compositor/buffer_stream_surfaces.h (+0/-1)
include/server/mir/frontend/session_mediator_report.h (+2/-0)
include/server/mir/graphics/display.h (+1/-1)
include/server/mir/graphics/display_report.h (+1/-1)
include/server/mir/graphics/platform.h (+3/-0)
include/server/mir/graphics/viewable_area.h (+1/-1)
include/server/mir/options/option.h (+1/-1)
include/server/mir/server_configuration.h (+1/-1)
include/server/mir/surfaces/buffer_stream.h (+2/-0)
include/test/mir_test_doubles/mock_android_alloc_device.h (+1/-0)
include/test/mir_test_doubles/mock_android_registrar.h (+1/-0)
include/test/mir_test_framework/testing_process_manager.h (+1/-0)
src/client/aging_buffer.h (+1/-1)
src/client/android/android_registrar.h (+2/-0)
src/client/client_platform.h (+2/-0)
src/server/compositor/rw_lock.h (+4/-0)
src/server/frontend/message_processor.h (+2/-2)
src/server/frontend/socket_session.cpp (+0/-1)
src/server/graphics/gbm/gbm_display.cpp (+3/-0)
src/server/graphics/gbm/gbm_platform.cpp (+0/-1)
tests/integration-tests/compositor/multithread_harness.h (+3/-0)
tests/unit-tests/client/android/test_client_android_registrar.cpp (+1/-0)
tests/unit-tests/graphics/android/test_android_framebuffer_window.cpp (+3/-0)
tests/unit-tests/test_gmock_fixes.cpp (+2/-0)
lp:~hikiko/mir/mir.fix-virt-destructorS
Rejected
for merging
into
lp:~mir-team/mir/trunk
- Alan Griffiths: Needs Fixing
- Daniel van Vugt: Needs Fixing
- PS Jenkins bot (community): Needs Fixing (continuous-integration)
-
Diff: 558 lines (+62/-25) (has conflicts)39 files modified3rd_party/cucumber-cpp/CTestTestfile.cmake (+2/-3)
3rd_party/cucumber-cpp/include/cucumber-cpp/internal/hook/HookRegistrar.hpp (+2/-0)
3rd_party/cucumber-cpp/include/cucumber-cpp/internal/hook/Tag.hpp (+1/-0)
3rd_party/cucumber-cpp/include/cucumber-cpp/internal/step/StepManager.hpp (+2/-0)
3rd_party/cucumber-cpp/src/CTestTestfile.cmake (+2/-2)
3rd_party/cucumber-cpp/src/connectors/wire/WireProtocol.cpp (+1/-0)
3rd_party/cucumber-cpp/tests/CTestTestfile.cmake (+2/-2)
3rd_party/cucumber-cpp/tests/utils/DriverTestRunner.hpp (+1/-0)
3rd_party/gmock-1.6.0/gtest/fused-src/gtest/gtest.h (+2/-1)
3rd_party/gmock-1.6.0/gtest/include/gtest/internal/gtest-param-util.h (+2/-1)
3rd_party/gmock-1.6.0/include/gmock/gmock-actions.h (+1/-1)
3rd_party/gmock-1.6.0/include/gmock/gmock-matchers.h (+1/-1)
CMakeLists.txt (+1/-1)
include/server/mir/compositor/buffer_allocation_strategy.h (+1/-2)
include/server/mir/compositor/buffer_bundle_surfaces.h (+1/-1)
include/server/mir/frontend/session_mediator_report.h (+2/-0)
include/server/mir/graphics/display.h (+1/-1)
include/server/mir/graphics/display_report.h (+1/-1)
include/server/mir/graphics/platform.h (+2/-0)
include/server/mir/graphics/viewable_area.h (+1/-1)
include/server/mir/options/option.h (+1/-1)
include/server/mir/server_configuration.h (+1/-1)
include/server/mir/surfaces/buffer_bundle.h (+2/-0)
include/test/mir_test_doubles/mock_android_alloc_device.h (+1/-0)
include/test/mir_test_framework/testing_process_manager.h (+1/-0)
src/client/android/android_client_buffer.cpp (+4/-0)
src/client/android/android_registrar.h (+2/-0)
src/client/client_platform.h (+2/-0)
src/client/rpc/mir_basic_rpc_channel.h (+1/-1)
src/client/rpc/mir_socket_rpc_channel.h (+6/-0)
src/server/frontend/message_processor.h (+2/-2)
src/server/frontend/socket_session.cpp (+0/-1)
src/server/graphics/gbm/gbm_display.cpp (+1/-0)
src/server/graphics/gbm/gbm_platform.cpp (+0/-1)
tests/integration-tests/compositor/multithread_harness.h (+3/-0)
tests/unit-tests/client/android/test_client_android_registrar.cpp (+2/-0)
tests/unit-tests/client/test_mir_connection.cpp (+2/-0)
tests/unit-tests/graphics/android/test_android_framebuffer_window.cpp (+1/-0)
tests/unit-tests/test_gmock_fixes.cpp (+1/-0)
lp:~hikiko/mir/mir.fix-missing-virtual-destructors
On hold
for merging
into
lp:~mir-team/mir/trunk
- PS Jenkins bot (community): Needs Fixing (continuous-integration)
- Robert Ancell: Needs Fixing
- Alan Griffiths: Needs Fixing
- Alexandros Frantzis (community): Needs Fixing
-
Diff: 1386 lines (+183/-29)55 files modified3rd_party/cucumber-cpp/CTestTestfile.cmake (+2/-3)
3rd_party/cucumber-cpp/include/cucumber-cpp/internal/hook/HookRegistrar.hpp (+2/-0)
3rd_party/cucumber-cpp/include/cucumber-cpp/internal/hook/Tag.hpp (+1/-0)
3rd_party/cucumber-cpp/include/cucumber-cpp/internal/step/StepManager.hpp (+2/-0)
3rd_party/cucumber-cpp/src/CTestTestfile.cmake (+2/-2)
3rd_party/cucumber-cpp/src/connectors/wire/WireProtocol.cpp (+1/-0)
3rd_party/cucumber-cpp/tests/CTestTestfile.cmake (+2/-2)
3rd_party/cucumber-cpp/tests/utils/DriverTestRunner.hpp (+1/-0)
3rd_party/gmock-1.6.0/gtest/fused-src/gtest/gtest.h (+2/-1)
3rd_party/gmock-1.6.0/gtest/include/gtest/internal/gtest-param-util.h (+2/-1)
3rd_party/gmock-1.6.0/include/gmock/gmock-actions.h (+24/-0)
3rd_party/gmock-1.6.0/include/gmock/gmock-generated-actions.h (+2/-0)
3rd_party/gmock-1.6.0/include/gmock/gmock-matchers.h (+44/-0)
include/server/mir/compositor/buffer_allocation_strategy.h (+1/-2)
include/server/mir/default_server_configuration.h (+1/-0)
include/server/mir/frontend/session_mediator_report.h (+2/-0)
include/server/mir/graphics/display.h (+1/-1)
include/server/mir/graphics/display_report.h (+1/-1)
include/server/mir/graphics/platform.h (+2/-0)
include/server/mir/graphics/viewable_area.h (+1/-1)
include/server/mir/logging/display_report.h (+1/-1)
include/server/mir/options/option.h (+1/-1)
include/server/mir/options/program_option.h (+1/-0)
include/server/mir/server_configuration.h (+1/-1)
include/server/mir/surfaces/buffer_bundle.h (+2/-0)
include/test/mir_test_doubles/mock_android_alloc_device.h (+1/-0)
include/test/mir_test_doubles/mock_display_report.h (+2/-0)
include/test/mir_test_doubles/mock_viewable_area.h (+1/-0)
include/test/mir_test_framework/testing_process_manager.h (+1/-0)
include/test/mir_test_framework/testing_server_configuration.h (+1/-0)
src/client/android/android_client_buffer.cpp (+4/-0)
src/client/android/android_registrar.h (+2/-0)
src/client/client_platform.h (+2/-0)
src/server/frontend/message_processor.h (+1/-1)
src/server/frontend/socket_session.cpp (+0/-1)
src/server/frontend/socket_session.h (+2/-0)
src/server/graphics/gbm/gbm_display.cpp (+2/-1)
src/server/graphics/gbm/gbm_display.h (+1/-1)
src/server/graphics/gbm/gbm_platform.cpp (+1/-2)
src/server/graphics/gbm/gbm_platform.h (+1/-1)
src/server/logging/display_report.cpp (+1/-1)
src/server/options/program_option.cpp (+4/-0)
tests/acceptance-tests/test_client_input.cpp (+4/-0)
tests/integration-tests/compositor/multithread_harness.h (+4/-3)
tests/integration-tests/frontend/test_application_mediator_report.cpp (+12/-0)
tests/integration-tests/test_display_server_main_loop_events.cpp (+4/-0)
tests/integration-tests/test_surface_first_frame_sync.cpp (+9/-1)
tests/integration-tests/test_surfaceloop.cpp (+6/-0)
tests/unit-tests/client/android/test_client_android_registrar.cpp (+2/-0)
tests/unit-tests/client/test_mir_connection.cpp (+4/-0)
tests/unit-tests/compositor/test_buffer_manager.cpp (+2/-0)
tests/unit-tests/frontend/test_session_mediator.cpp (+2/-0)
tests/unit-tests/graphics/android/test_android_framebuffer_window.cpp (+1/-0)
tests/unit-tests/shell/test_consuming_placement_strategy.cpp (+2/-0)
tests/unit-tests/test_gmock_fixes.cpp (+2/-0)
summary: |
- serious bug that might be causing memory leaks + serious bug that might cause memory leaks |
Changed in mir: | |
assignee: | nobody → Eleni Maria Stea (hikiko) |
status: | New → In Progress |
Changed in mir: | |
importance: | Low → Medium |
Changed in mir: | |
status: | In Progress → Fix Committed |
status: | Fix Committed → In Progress |
Changed in mir: | |
milestone: | none → 0.0.4 |
Changed in mir: | |
milestone: | 0.0.4 → 0.0.5 |
Changed in mir: | |
status: | Fix Committed → Fix Released |
To post a comment you must log in.
We only use such classes through shared_ptrs, which, even when upcasted, "remember" the derived class type and call the proper destructor.
Of course, that's not to say that we shouldn't fix the missing virtual destructors (we should!), just that they are not a super critical issue in our case.