OpenTTD downloading from OneDrive freezes at startup

[perezdidac]

Version of OpenTTD

1.11

Windows 10

Expected result

OpenTTD starts

Actual result

OpenTTD starts, black screen, frozen cursor.

Thread blocked at

FILE *f = fopen(filename.c_str(), "rb");

in fileio.cpp.

filename is the name of the file being downloaded.

Observe OneDrive downloading stuff (I have no idea why) in Documents/OpenTTD:

That thread holds the game_state_mutex here:

So the main thread blocked at game_state_mutex:

and that's why we are frozen.

But the key here is that the fopen blocks until the file has been downloaded.

Steps to reproduce

I don't know why OneDrive is downloading content...

[nielsmh]

Your own screenshot has the answer to why OneDrive is involved: The path to your Documents folder is inside your OneDrive sync folder. As to why your files are not cached locally, I nobody here can answer that.
The same hang would happen if the files were on a hard disk drive that was spun down and needed to spin up before being able to read the file.

[PeterN]

We could possibly test the file attributes to see if it's marked offline. However that might then end up with reports about files being ignored...

[nielsmh]

A better solution would be if all the file work was done in another thread than the UI, so the UI at least stays responsive if any file operation is slow.

[TrueBrain]

A better solution would be if all the file work was done in another thread than the UI, so the UI at least stays responsive if any file operation is slow.

Some background for you all, so you get a bit more feeling where OpenTTD currently stands in terms of threading. As I think you mean the same as I am about to write, but the way you think about the solution might put you on the wrong track. Hopefully the following is going to help you out a bit:

At the moment, when using threads, there are always two threads worth mentioning:

1. GameThread, handling the GameLoop once in a while (and the one hanging here)
2. MainThread (or DrawThread), doing the UI drawing

Between those threads is a lock on the game-state. While the game-state is locked by either of the thread, the other cannot modify it in any way (for obvious reasons). How we currently make the NewGRF scanner responsive, is by releasing the lock on the game-state after each NewGRF that is loaded, giving the DrawThread a chance to make something appear on the screen (and some clever mechanism in the video-drivers makes sure they both get a turn).

In this instance, fopen is blocking, while having a lock on the game-state. In result, the DrawThread cannot produce a single frame, giving the appearance of the game being frozen.

A solution could be that we release the game-state a lot more often. For example, a lot in that function doesn't change the game-state at all. So if it would to release its lock on the game-state, the UI would happily continue to do its thing, giving the appearance everything is not hanging. The game itself of course still is, as we cannot continue till the scan is done anyway. But the mouse will be responsive, you can move windows, etc.

This is something new that we introduced in 1.11, and where we just started to get a feel of what we can and cannot do. It has a long way to go before we have a good feeling with it, but basically, locking the game-state less often will help enormously with how the game feels in terms of responsiveness. We also have the problem the other way around, that drawing the viewports keeps a lock on the game-state for far too long, meaning the next game-tick cannot happen because we are drawing. There is absolutely no need for that. But I digress.

The trick with releasing the game-state lock is that you don't want to do it too often, and lose performance on silly context-switches. So if someone would to look into this, a good look into what the GRF Scanner is doing and how / when it influence the game-state is a good first step. For example, in the viewport I noticed that if I add a small queue, I could easily do a lot of calculations outside the game-state lock. At the expensive of a bit of memory. But that is often a trade-of. I did not look into this specific function in detail, but I can imagine similar things going on here. Things like MD5 calculations, opening of file, reading of file, etc, all do not influence the game-state. If we can do all that first, while we do not hold a lock on the game-state, the NewGRF scanner will feel a lot more snappy.

Anyway, long story, sorry about that .. I hope it helps out in understanding a bit what a possible direction for a solution could be :)

[perezdidac]

Wow thanks for the answers. I totally agree with @TrueBrain's one, this is a classic programming mistakes in all GUI apps/programs in general.

The main reason I wanted to raise this topic is because if I am experiencing this there is a high likelihood that others are, at least players with a plain Windows 10 installation as this OneDrive thing is on by default AFAIK.

As mentioned in above's comments, GameLoop() is executed after locking the game_state_mutex. GameLoop() then does lots of things obviously, one of them being ScanNewGRFFiles(), which is the one that ends up blocking here. I am not familiar with whether that can run safely without locking the game_state_mutex, but if it does that will make the fix straightforward, but I doubt it is safe as per now...