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

Jump to bottom

Fix: Memory leak of airport tile layout in AirportChangeInfo (prop 0A) #8928

Merged
merged 1 commit into from Apr 2, 2021
Merged

Fix: Memory leak of airport tile layout in AirportChangeInfo (prop 0A) #8928

merged 1 commit into from Apr 2, 2021

Conversation

JGRennison
Copy link
Contributor

Motivation / Problem

This fixes the long-standing memory leak in AirportChangeInfo (prop 0A) introduced by 917d4cb.
This leak is entirely harmless, so I'd ignored it until now, but having it logged every time is a bit annoying, so it's fixed now.

Use an airport GRF such as OpenGFX+ Airports to reproduce the leak.

Leak details 
Direct leak of 112 byte(s) in 14 object(s) allocated from:
  #0 0x7f89e2c63bc8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
  #1 0x5589d45ec387 in MallocT<AirportTileTable*> /home/jgr/misc/openttd-5/src/core/alloc_func.hpp:69
  #2 0x5589d458e066 in DuplicateTileTable /home/jgr/misc/openttd-5/src/newgrf.cpp:3779
  #3 0x5589d458f87f in AirportChangeInfo /home/jgr/misc/openttd-5/src/newgrf.cpp:3859
  #4 0x5589d4599452 in FeatureChangeInfo /home/jgr/misc/openttd-5/src/newgrf.cpp:4770
  #5 0x5589d45de457 in DecodeSpecialSprite /home/jgr/misc/openttd-5/src/newgrf.cpp:9305
  #6 0x5589d45df6fa in LoadNewGRFFile(GRFConfig*, unsigned int, GrfLoadingStage, Subdirectory) /home/jgr/misc/openttd-5/src/newgrf.cpp:9427
  #7 0x5589d45e9fe8 in LoadNewGRF(unsigned int, unsigned int, unsigned int) /home/jgr/misc/openttd-5/src/newgrf.cpp:9838
  #8 0x5589d41d3bb8 in LoadSpriteTables /home/jgr/misc/openttd-5/src/gfxinit.cpp:230
  #9 0x5589d41d5c2c in GfxLoadSprites() /home/jgr/misc/openttd-5/src/gfxinit.cpp:338
  #10 0x5589d40d9543 in GenerateWorld(GenWorldMode, unsigned int, unsigned int, bool) /home/jgr/misc/openttd-5/src/genworld.cpp:306
  #11 0x5589d4a3f7d9 in MakeNewGame /home/jgr/misc/openttd-5/src/openttd.cpp:923
  #12 0x5589d4a402f4 in SwitchToMode(SwitchMode) /home/jgr/misc/openttd-5/src/openttd.cpp:1054
  #13 0x5589d4a49c98 in GameLoop() /home/jgr/misc/openttd-5/src/openttd.cpp:1487
  #14 0x5589d3bb5f44 in VideoDriver::GameLoop() /home/jgr/misc/openttd-5/src/video/video_driver.cpp:36
  #15 0x5589d3bb6240 in VideoDriver::GameThread() /home/jgr/misc/openttd-5/src/video/video_driver.cpp:43
  #16 0x5589d3bb693a in VideoDriver::GameThreadThunk(VideoDriver*) /home/jgr/misc/openttd-5/src/video/video_driver.cpp:80
  #17 0x5589d3bbd09e in StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}::operator()(char const*, void (*&&)(VideoDriver*), VideoDriver*&&) const /home/jgr/misc/openttd-5/src/video/../thread.h:54
  #18 0x5589d3bc083c in void std::__invoke_impl<void, StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}, char const*, void (*)(VideoDriver*), VideoDriver*>(std::__invoke_other, StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&&&)(VideoDriver*), VideoDriver*&&)#1}, char const*&&, void (*&&)(VideoDriver*), VideoDriver*&&) /usr/include/c++/9/bits/invoke.h:60
  #19 0x5589d3bc03fa in _ZSt8__invokeIZ14StartNewThreadIPFvP11VideoDriverEJS2_EEbPSt6threadPKcOT_DpOT0_EUlS8_OS4_OS2_E_JS8_S4_S2_EENSt15__invoke_resultIS9_JDpSB_EE4typeESA_SD_ /usr/include/c++/9/bits/invoke.h:95
  #20 0x5589d3bc005f in void std::thread::_Invoker<std::tuple<StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}, char const*, void (*)(VideoDriver*), VideoDriver*> >::_M_invoke<0ul, 1ul, 2ul, 3ul>(std::_Index_tuple<0ul, 1ul, 2ul, 3ul>) /usr/include/c++/9/thread:244
  #21 0x5589d3bbfdac in std::thread::_Invoker<std::tuple<StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}, char const*, void (*)(VideoDriver*), VideoDriver*> >::operator()() /usr/include/c++/9/thread:251
  #22 0x5589d3bbfd62 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}, char const*, void (*)(VideoDriver*), VideoDriver*> > >::_M_run() /usr/include/c++/9/thread:195
  #23 0x7f89e2244d83  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xd6d83)

Indirect leak of 2010 byte(s) in 14 object(s) allocated from:
  #0 0x7f89e2c63bc8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
  #1 0x5589d45ec3f5 in MallocT<AirportTileTable> /home/jgr/misc/openttd-5/src/core/alloc_func.hpp:69
  #2 0x5589d458e2b6 in DuplicateTileTable /home/jgr/misc/openttd-5/src/newgrf.cpp:3786
  #3 0x5589d458f87f in AirportChangeInfo /home/jgr/misc/openttd-5/src/newgrf.cpp:3859
  #4 0x5589d4599452 in FeatureChangeInfo /home/jgr/misc/openttd-5/src/newgrf.cpp:4770
  #5 0x5589d45de457 in DecodeSpecialSprite /home/jgr/misc/openttd-5/src/newgrf.cpp:9305
  #6 0x5589d45df6fa in LoadNewGRFFile(GRFConfig*, unsigned int, GrfLoadingStage, Subdirectory) /home/jgr/misc/openttd-5/src/newgrf.cpp:9427
  #7 0x5589d45e9fe8 in LoadNewGRF(unsigned int, unsigned int, unsigned int) /home/jgr/misc/openttd-5/src/newgrf.cpp:9838
  #8 0x5589d41d3bb8 in LoadSpriteTables /home/jgr/misc/openttd-5/src/gfxinit.cpp:230
  #9 0x5589d41d5c2c in GfxLoadSprites() /home/jgr/misc/openttd-5/src/gfxinit.cpp:338
  #10 0x5589d40d9543 in GenerateWorld(GenWorldMode, unsigned int, unsigned int, bool) /home/jgr/misc/openttd-5/src/genworld.cpp:306
  #11 0x5589d4a3f7d9 in MakeNewGame /home/jgr/misc/openttd-5/src/openttd.cpp:923
  #12 0x5589d4a402f4 in SwitchToMode(SwitchMode) /home/jgr/misc/openttd-5/src/openttd.cpp:1054
  #13 0x5589d4a49c98 in GameLoop() /home/jgr/misc/openttd-5/src/openttd.cpp:1487
  #14 0x5589d3bb5f44 in VideoDriver::GameLoop() /home/jgr/misc/openttd-5/src/video/video_driver.cpp:36
  #15 0x5589d3bb6240 in VideoDriver::GameThread() /home/jgr/misc/openttd-5/src/video/video_driver.cpp:43
  #16 0x5589d3bb693a in VideoDriver::GameThreadThunk(VideoDriver*) /home/jgr/misc/openttd-5/src/video/video_driver.cpp:80
  #17 0x5589d3bbd09e in StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}::operator()(char const*, void (*&&)(VideoDriver*), VideoDriver*&&) const /home/jgr/misc/openttd-5/src/video/../thread.h:54
  #18 0x5589d3bc083c in void std::__invoke_impl<void, StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}, char const*, void (*)(VideoDriver*), VideoDriver*>(std::__invoke_other, StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&&&)(VideoDriver*), VideoDriver*&&)#1}, char const*&&, void (*&&)(VideoDriver*), VideoDriver*&&) /usr/include/c++/9/bits/invoke.h:60
  #19 0x5589d3bc03fa in _ZSt8__invokeIZ14StartNewThreadIPFvP11VideoDriverEJS2_EEbPSt6threadPKcOT_DpOT0_EUlS8_OS4_OS2_E_JS8_S4_S2_EENSt15__invoke_resultIS9_JDpSB_EE4typeESA_SD_ /usr/include/c++/9/bits/invoke.h:95
  #20 0x5589d3bc005f in void std::thread::_Invoker<std::tuple<StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}, char const*, void (*)(VideoDriver*), VideoDriver*> >::_M_invoke<0ul, 1ul, 2ul, 3ul>(std::_Index_tuple<0ul, 1ul, 2ul, 3ul>) /usr/include/c++/9/thread:244
  #21 0x5589d3bbfdac in std::thread::_Invoker<std::tuple<StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}, char const*, void (*)(VideoDriver*), VideoDriver*> >::operator()() /usr/include/c++/9/thread:251
  #22 0x5589d3bbfd62 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<StartNewThread<void (*)(VideoDriver*), VideoDriver*>(std::thread*, char const*, void (*&&)(VideoDriver*), VideoDriver*&&)::{lambda(char const*, void (*&&)(VideoDriver*), VideoDriver*&&)#1}, char const*, void (*)(VideoDriver*), VideoDriver*> > >::_M_run() /usr/include/c++/9/thread:195
  #23 0x7f89e2244d83  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xd6d83)

Description

When replacing AirportSpec::table in AirportChangeInfo (prop 0A), free the old tables instead of leaking them.

Limitations

N/A

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@LordAro LordAro added the backport requested This PR should be backport to current release (RC / stable) label Apr 2, 2021
@michicc michicc merged commit 83ac5aa into OpenTTD:master Apr 2, 2021
LordAro pushed a commit to LordAro/OpenTTD that referenced this pull request Apr 17, 2021
@JGRennison @LordAro
Fix: Memory leak of airport tile layout in AirportChangeInfo (prop 0A) (
1788d99 
OpenTTD#8928)
LordAro pushed a commit that referenced this pull request Apr 18, 2021
@JGRennison @LordAro
Fix: Memory leak of airport tile layout in AirportChangeInfo (prop 0A) (
34606b7 
#8928)
@LordAro LordAro added backported This PR is backported to a current release (RC / stable) and removed backport requested This PR should be backport to current release (RC / stable) labels Apr 18, 2021
@JGRennison JGRennison deleted the fix-airport-layout-leak branch January 9, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LordAro LordAro LordAro approved these changes

Assignees
No one assigned
Labels
backported This PR is backported to a current release (RC / stable)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JGRennison @LordAro @michicc