Parsing problem in a DDS file at mission start

[grayman]

I occasionally get this error when starting a mission using SVN.

I'm using the 16x9tv display mode. It looks like some parser is expecting something other than the legal start of a dds file header.

[duzenko]

Never seen this

How often does this happen? Could you set a breakpoint on line 536 in MapFile.cpp and provide the stack trace?

From the error description it seems to try to load the .dds file as game map file which does not make sense.

Is it your WIP?

[grayman]

Yes, my WIP.

I tried "The Bakery Job" with a build of the latest SVN sources, and this happened:

 

bakery.map certainly doesn't contain "unknown punctuation".

 

These issues don't happen all the time.

I'll try with the debugger suggestion.

 

[stgatilov]

Hmm... I guess I have seen the second error recently. Don't remember how it ended.

[grayman]

Loaded "Business As Usual" and repeatedly crash with the following:

business as usual.txt

 

[grayman]

The BaU crash happens with both 32- and 64-bit binaries.

Works fine in 2.07 hotfix.

[grayman]

I'm unable to get The Bakery Job to fail again.

Might try again later.

[duzenko]

This looks more like memory corruption than anything else

[duzenko]

I got a random AV when starting mission download in idStr::EnsureAlloced that I can't repeat so the error msg could be irrelevant

[grayman]

Any more thoughts on this?

The latest SVN Windows exes are crapping out for me on these parsing problems. Tried The Gatehouse and Business as Usual (again).

Becoming an issue because I can't rely on being able to run SVN for anything: playing the game or working on WIPs. I've shelved work on the Steele WIPs because from here on they rely on 2.08+. Just to keep working, I've started a new non-Steele WIP that relies on 2.07.

 

Thanks.

[duzenko]

@stgatilov

Is there any debug tool that would allow to see where a pointer has been freed?

@grayman

Is it the same stacktrace each time it crashes? And it crashes aat map start all the time?

[stgatilov]

Grayman, please attach your darkmod.cfg. I will try to reproduce.

As for tools, I don't know any silver bullet. Linux had valgrind. I have also used Heapy on Windows, it's easy to customize.

UPDATE: Also, a crashdump would be helpful.

[duzenko]

We had a build config called 'memory log' or something. What was it supposed to do?

[stgatilov]

8 minutes ago, duzenko said:

We had a build config called 'memory log' or something. What was it supposed to do?

Well, it enables various defines.

You can see the defines in BuildDefines.h. There also a bit of writeup here, but it is more focused on TypeInfo, which is not necessary.

You can temporarily uncomment ID_DEBUG_MEMORY and probably ID_REDIRECT_NEWDELETE in Debug build, that should be enough to enable whatever debug heap which is provided by ID code. And it won't require typeinfo as the "with memory log" requires right now.

 

However, note that TDM was developed by many people over 15 years, and nobody cared about such tools. So don't be surprised to get all sort of problems with them. I guess people often directly use malloc/free and operator new/operator delete for allocating memory. The define ID_REDIRECT_NEWDELETE is a very hacky attempt to fix that, but it probably does not influence STL usage, etc. Some universal tool may be more precise.

[stgatilov]

One of the things which came to my mind is string pools.

All idDict-s store keys and values inside a string pool. If you access same string pool from many threads, that is a problem. The correct approach would be to have several pools indexed by thread ID or TLS index. Another way is wrapping all string pool functions into a critical section, but I think it is inferior. Anyway, unless I can see the crash, I cannot say anything for sure.

[duzenko]

There's a stack trace above

[stgatilov]

1 hour ago, duzenko said:

There's a stack trace above

That's not much. But stack trace confirms the idStrPool theory: it crashes on access to a string inside idDict. A crashdump will help, because it embeds stacks of all threads, and the state of heap, where it is sometimes possible to notice something.

Config file is the most wanted thing here. I won't be surprised that with right settings, we will reproduce the problem on our machines.

[duzenko]

I see

Well, this happens during map load

Not much happening in threads at that time

[grayman]

7 hours ago, stgatilov said:

Grayman, please attach your darkmod.cfg. I will try to reproduce.

 

Attached.

Today was an unplanned hospital day. I'll try to answer any other questions tomorrow.

Darkmod.cfg

[stgatilov]

Ok, I have reproduced the problem. Although I see nothing suspicious in config. I'll commit automation script to assets SVN shortly. You can run it is python repeat.py enter_game_exit.seq from the directory where you see the .seq-file.

This is heap corruption due to multithreading. The newly introduced "background image loading" thread loads a TGA file of menu background. Somewhere during loading, it gets a path "C:\TheDarkMod\darkmod\fms\business3\guis\assets\loading_screen.tga". In the main thread, this string somehow gets into a local idStr of idFileSystemLocal::OpenFileReadFlags. When this string is destroyed, it notices that someone wrote past the end of buffer.

So we have to review the whole idStr for working properly with multithreading.

[stgatilov]

I think the main question to Duzenko is: why is background thread doing anything at all?

I recall you said that the background thread is used only when cvar is enabled. I don't see suspicious cvar values in the config file.

UPDATE: I think first of all background thread should be fixed: it must be trivial to validate that it does absolutely nothing or is not spawned at all.

[duzenko]

4 hours ago, stgatilov said:

I think the main question to Duzenko is: why is background thread doing anything at all?

I recall you said that the background thread is used only when cvar is enabled. I don't see suspicious cvar values in the config file.

UPDATE: I think first of all background thread should be fixed: it must be trivial to validate that it does absolutely nothing or is not spawned at all.

If idStr is not thread safe then the we need to fix it. Yeah the imageloading thread must not do anything with the cvar disabled but that's not the problem here.

[duzenko]

Do you think maybe it's idFileSystemLocal  or idLexer that is unsafe?

Are you running TDM with pk4's or uncompressed? It could be the pak linked list in FileSystem

idFileSystemLocal::BuildOSPath >> static char OSPath?

Quote

this string somehow gets into a local idStr of idFileSystemLocal::OpenFileReadFlags

What local string? What is its C++ identifier? The only local idStr I can see - netpath - is not used for files in pk4's.

BTW still not able to repeat it.

[stgatilov]

7 hours ago, duzenko said:

If idStr is not thread safe then the we need to fix it. Yeah the imageloading thread must not do anything with the cvar disabled but that's not the problem here.

I hope you will fix it in some nearest future.

Yes, idStr better be thread-safe. I'll look into it, of course.

As for idDict and idStrPool... I'm not certain. Looking at the D3BFG code, they did not make custom allocators thread-safe. I see no atomic operations and mutexes in Heap.h/cpp, and non of that in idStrPool and idDict. Making all that stuff thread-safe will make it significantly slower. I guess they simply fixed stupid non-thread-safety like static and global variables, and ensured that access to custom allocators and string pool is single-threaded (e.g. from game code only).

 

Quote

still not able to repeat it.

Did you try to run the repeat.py script? I ran it directly on SVN with Win64 Debug build. I cannot repeat the debug heap dialog message, since most of the time TDM process simply "disappears" somewhere during loading. I assume it is a crash, but I have disabled something like WER or JIT in my system, so I don't see the message box. I guess I should reenable it somehow.

[duzenko]

I tried from inside the VS debugger and it just works each time, debug and release