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

Crash of OpenTTD master gf81cb0a90d possibly after trying to sell the default human user #7737

Closed
Wormnest opened this issue Sep 12, 2019 · 10 comments

Comments

@Wormnest
Copy link

Version of OpenTTD: master gf81cb0a90d.

OS: Windows 10 Home 64 bits build 1809.

Expected result: Continue running.

Actual result: Crash.

crash.log
crash

It seems like GitHub doesn't like the crash.sav and crash.dmp even if I zip them up. Maybe they are too big? crash.dmp is about 20 MB.

Steps to reproduce.

Started a test run with myself and 1 AI: the development version of my AI WormAIDev testing only trains for computer players, max trains 50.
Started in 1980 and let it run for a long time. Me (the human user didn't do any building at all).
Around 2147 it crashes and it looks like it possibly was trying to sell the human user company.
Same or similar looking crash happened on a previous master from around Sept 5 around the same date.

@glx22
Copy link
Contributor

glx22 commented Sep 12, 2019

I quickly checked crash.log and indeed crash.dmp will be needed.
According to https://help.github.com/en/articles/file-attachments-on-issues-and-pull-requests zipped crash.dmp should be ok.

@Wormnest
Copy link
Author

When trying to upload crash.zip with the crash.dmp in it it says We don’t support that file type. Try again with a GIF, JPEG, JPG, PNG, DOCX, GZ, LOG, PDF, PPTX, TXT, XLSX or ZIP.
I renamed it to crashdmp.gz (but it is a zip) and that seems to have worked.

crashdmp.gz

@glx22
Copy link
Contributor

glx22 commented Sep 12, 2019

>	openttd.exe!NewsWindow::DrawWidget(const Rect & r, int widget) Ligne 396	C++
 	openttd.exe!NWidgetLeaf::Draw(const Window * w) Ligne 2520	C++
 	openttd.exe!NWidgetPIPContainer::Draw(const Window * w) Ligne 1113	C++
 	openttd.exe!NWidgetPIPContainer::Draw(const Window * w) Ligne 1113	C++
 	openttd.exe!NWidgetPIPContainer::Draw(const Window * w) Ligne 1113	C++
 	openttd.exe!NWidgetBackground::Draw(const Window * w) Ligne 1865	C++
 	openttd.exe!NWidgetPIPContainer::Draw(const Window * w) Ligne 1113	C++
 	openttd.exe!Window::DrawWidgets() Ligne 608	C++
 	openttd.exe!DrawOverlappedWindow(Window * w, int left, int top, int right, int bottom) Ligne 933	C++
 	openttd.exe!DrawOverlappedWindowForAll(int left, int top, int right, int bottom) Ligne 951	C++
 	openttd.exe!RedrawScreenRect(int left, int top, int right, int bottom) Ligne 1293	C++
 	openttd.exe!DrawDirtyBlocks() Ligne 1387	C++
 	openttd.exe!UpdateWindows() Ligne 3185	C++
 	openttd.exe!VideoDriver_Win32::MainLoop() Ligne 1284	C++
 	openttd.exe!openttd_main(int argc, char * * argv) Ligne 862	C++
 	openttd.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Ligne 444	C++
 	[Code externe]	

@Wormnest
Copy link
Author

I can reproduce this without any AI too. Just start a new map (I used 512x512 now, start in 1980), don't build anything and use fast forward.
This one crashes in 2062 just when the transport company in trouble message starts popping up.
crash.log
crash
crash.dmp.gz
This time it really is a gz and not a renamed zip.

@glx22
Copy link
Contributor

glx22 commented Sep 12, 2019

I think I see what happens, but my system seems too slow to reproduce it.
It seems the "Company in trouble" NewsItem is deleted while the news is still displayed, causing the invalid read.

@glx22
Copy link
Contributor

glx22 commented Sep 12, 2019

A probable faster way to trigger it is to disable inflation and set max loan to the minimum possible value, then when the game starts spend all money in terraforming.

@Eddi-z
Copy link
Contributor

Eddi-z commented Sep 12, 2019

Can't you just cheat -1000000...00 money?

@Eddi-z
Copy link
Contributor

Eddi-z commented Sep 12, 2019

just out of curiosity, can you reproduce that without afbf6a5?

@Milek7
Copy link
Contributor

Milek7 commented Sep 12, 2019

I think this fixes it:

diff --git a/src/news_gui.cpp b/src/news_gui.cpp
index 34cfa7a58..3b9b0fca9 100644
--- a/src/news_gui.cpp
+++ b/src/news_gui.cpp
@@ -676,7 +676,7 @@ static void MoveToNextNewsItem()
 
                        case ND_FULL: // Full - show newspaper
                                ShowNewspaper(ni);
-                               break;
+                               return;
                }
        }
 }

@Eddi-z
Copy link
Contributor

Eddi-z commented Sep 12, 2019

there's two other breaks in the other function that should also be returns

Milek7 added a commit to Milek7/OpenTTD that referenced this issue Sep 12, 2019
Eddi-z added a commit to Eddi-z/OpenTTD that referenced this issue Sep 12, 2019
douiwby pushed a commit to douiwby/OpenTTD that referenced this issue Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants