Strange CTD on cityconquest

AbsintheRed

Deity
Joined
Jul 27, 2009
Messages
8,288
Location
Szeged, Hungary
I'm struggling with a CTD in RFC Europe for some time now, and it is a strange one. I'm running out of ideas where to look.
If anyone could point me in the right direction, all the RFCE community would be very grateful.

Here is the thread for the latest version of the mod, you will find dl links for version 1.3 and the 1.3.1 patch there: http://forums.civfanatics.com/showthread.php?t=564554
Alternatively you can use the latest SVN version (revision 1193 or revision 1194, check this thread for SVN).
1.3.1 and revision 1193-94 use the same .dll.
Source files are included in the mod, and I also uploaded the debug .dll for this version: https://sourceforge.net/projects/rfceurope/files/CvGameCoreDLL.dll/download

The CTD in question always occurs on conquering a city. The call stack points to isDead in CvUnit.cpp, and to the getDamage function.
To be more precise, the end of it is in CvUnit::canMove() -> CvUnit::isDead() -> CvUnit::getDamage(), in every CTD dump file I have checked.
The CTD itself is a memory access failure: "0xC0000005: Access violation reading location 0x00000038."

The strangest thing in it that it doesn't appear on all systems. There are differences how often does it come up on Windows 7, 8.1, and 10.
Attached a savegame with the corresponding version of the mod. It crashes after hitting end turn.
 

Attachments

I also want to point out, that there are some assert failures in the mod.
Most of them are harmless/intentional though, connected to RFC-specific stuff (like independent nations, settler/war maps, events which gives you units you couldn't build otherwise)

If you find any suspicious stuff connected to asserts though, pls also tell me.
I already fixed 2-3 of them in the last few weeks, but it's entirely possible I labeled something as harmless when in fact it hides an issue connected to the CTD.

EDIT: here are all the relevant files on sourceforge: https://sourceforge.net/projects/rfceurope/files/?source=navbar
 
Downloading (slowly :undecide:) from svn. I will have a look when I actually have the files. For once such a request for help actually includes a link to the source code itself. That's a great improvement over the usual cries for help :)

The CTD in question always occurs on conquering a city. The call stack points to isDead in CvUnit.cpp, and to the getDamage function.
To be more precise, the end of it is in CvUnit::canMove() -> CvUnit::isDead() -> CvUnit::getDamage(), in every CTD dump file I have checked.
The CTD itself is a memory access failure: "0xC0000005: Access violation reading location 0x00000038."
Sounds like a using a unit pointer, which then turns out to be null. 0x38 is actually 0x38+pointer, which in turn is the address of a variable for the class in question. If it's really needed, it is possible to read CvUnit.h and figure out which variable is at that address, but presumably it will be a whole lot easier to just ask the debugger what goes on.

The strangest thing in it that it doesn't appear on all systems. There are differences how often does it come up on Windows 7, 8.1, and 10.
The only reason I can think of a difference would be due to how the different OSes handles memory allocation. The next obvious question is why does this matter? If the code is ok, it shouldn't matter at all. I will not rule out the possibility that a new unit or city is attempted to be allocated, then fails to allocate due to low memory. This will create a null pointer where a non-null pointer is expected.

I'm using windows 7 btw. For once that info seems to matter.
 
If you prefer that way, I can upload only the source files as well.
So you don't have to download the whole mod.
On the other hand, it might be better this way if you want to have a look on the savegame as well.
Huge thanks for taking a look at it!
 
Sounds like a using a unit pointer, which then turns out to be null. 0x38 is actually 0x38+pointer, which in turn is the address of a variable for the class in question. If it's really needed, it is possible to read CvUnit.h and figure out which variable is at that address, but presumably it will be a whole lot easier to just ask the debugger what goes on.

Yeah, it's probably a pointer error. Unfortunately I'm not that experienced in debugging, so i didn't succeed in figuring the cause out :(
Interesting info about the CvUnit.h, will try to look at it in the meantime.

The only reason I can think of a difference would be due to how the different OSes handles memory allocation. The next obvious question is why does this matter? If the code is ok, it shouldn't matter at all. I will not rule out the possibility that a new unit or city is attempted to be allocated, then fails to allocate due to low memory. This will create a null pointer where a non-null pointer is expected.

Yep, this is what I suspect as well.
The fact that it's OS-dependent definitely points in the direction of memory handling.
Also, on some rare occasions, the CTD won't even happen on systems where it usually occurs. I had one savegame which I was testing various stuff with (connected to the crash) so I ran it quite a lot. It resulted in a CTD in 95% of the time, but went through in 1-2 cases.
So we can be quite sure about this.

I'm using windows 7 btw. For once that info seems to matter.

I'm on 8.1, it crashes for me.
Funny thing to say, but I really hope it crashes for you too :lol:
If not, I will upload some other savegames as well, I already asked for them in the subforum.
 
My plan is to test the savegame, which mean I have to download everything. Besides I prefer to have the files in svn. If I start to make changes and figures out a way to make it stable, svn will remind me what I changed. I usually use git for that, but svn is not bad either.

Yeah, I also love SVN, I always use it for my projects. Makes life so much easier, for multiple reasons.
Some offtopic while you are waiting for the files: Is git better? Why do you prefer that over svn?
 
I prefer git over svn because SmartGIT is the best version control client I have found so far, both technically and for UI. It makes working with branches and checking diffs and log and stuff much easier. Also git clones the entire server to your HD, meaning it can do everything offline. It means less waiting time when interacting with the log and it is extremely useful if you mod/code using a poor or no internet connection. Particularly the "works offline" was the initial reason why I decided to use git.

I moved M:C from github to sourceforge quite easily due to the cloned server setup. I just edited the remote server URL and hit push (upload) and everything ended up on the new server. (SF has unlimited space, github doesn't for free accounts). Git can even handle multiple servers where you merge data from them. I haven't used that much, but it allows forking a mod, write to your new repository and then tell the original owner about your patch. In other words you can commit without write access and the owner of the original can review and accept it or not. Quite useful for accepting outside patches.

It's possible to give smartGIT the link to an svn server. It will then download every single revision from that server and turn it into a git repository. Going from git to svn is more of an issue and I'm not even sure if it is possible.
 
This is very confusing... I've tried to reproduce the crashes reported by the people in the RFCEurope forum in the latest week or so, but haven't been able to get it to crash. I'm almost starting to suspect that I've contaminated my environment somehow. Maybe I should restart my debugging in a clean install of Civ.

Nightinggale said:
I prefer git over svn because SmartGIT is the best version control client I have found so far, both technically and for UI. It makes working with branches and checking diffs and log and stuff much easier. Also git clones the entire server to your HD, meaning it can do everything offline. It means less waiting time when interacting with the log and it is extremely useful if you mod/code using a poor or no internet connection. Particularly the "works offline" was the initial reason why I decided to use git.

+1. After having used Git for the last 6 years or so I have little good to say about SVN.

Let me know if you want assistance with moving the project to Git: I've done that before (professionally).
 
I can't get it to crash, but I have some interesting findings anyway.

In CvPlayer::read()
Code:
reset([B]getID()[/B]);
Adding the ID kills some asserts. I'm not sure if they matter, but it's always good to remove assert failures.

There are asserts about getPower(). This function returns the cached power of a player/team. However it seems that the cache is invalid somehow. It seems the problem is that the power is negative, which would be quite bad. Using the wrong positive number just makes the AI misjudge how powerful the player is, but a negative number might cause other issues since the code assumes it to never be negative.

The AI is confused about a selection group and still fails to figure out anything to do with it after 100 attempts. I think the result of this is the same as hitting space, that is the selection group will not do anything that turn.

Team 5 is at war with team 30, but team 30 is not at war with team 5. I have no idea how that happened, but it is a very serious bug and must be fixed ASAP regardless of its relationship with the potential crash.
 
Thanks for checking, hopefully I can provide some different (potentially) crashing savegames soon!

There are independent civs in RFCE. Small, non-playable nations, mostly there for the normal civs to conquer them.
AFAIK the getPower function throws on assert on these civs, which is not necessarily a bad thing. It's probably from Rhye, and the point is that the AI almost always want to declare war on indy civs.
I'm not sure about the negative part, I originally assumed that this is harmless.
You are probably right though, this is also something to check in detail.

Team 30 is an indy civ, again, so with special rules.
It is definitely a bug though that it's not mutually at war with a major civ.
I have no clue about the selection group... Do you have any more detail about these?
 
Not at the moment. I just noticed it had a problem and then I ignored it due to hunting for a crash. I could go back and investigate, but it will have to wait. My mind isn't in bughunting mode right now.

Sure, take your time. No rush at all.
I appreciate every help I can get on these, so thanks again :)
 
Got a new CTD savegame from another player.
Hopefully it will crash for you too (it does for me):
 

Attachments

I figured out what cause the AI code to get stuck on a unit.

The code design is that the AI runs through some checks to figure out what to do with a unit. It's in a loop, which mean it will keep going until the unit reports it can't move anymore or fortified or similar. The loop makes sense when you think of a ship exploring. The AI orders it to move one plot, then it reveals some more plots and it will rerun the checks to figure out where to move it next. Because of cases like this, it makes sense to loop until it has used all movement points.

The assert happens in this loop. Some vanilla code detects that it has run 100 times and still haven't finished the unit for the turn. It then asserts and ends the loop. Odds are that it is stuck in the loop and exiting will prevent the game from freezing.

The bug is in AI_unitUpdate. If it returns True, it will only use the python code to determine what to do with the unit while False will make the unit use the DLL code as well. What happens is it runs into a prosecutor and the python code fails to find a city for it. Even in that case it returns true, which mean it will not be asked to do anything from either python or the DLL. When it does nothing, it really does get stuck in the loop because it keeps reporting ready to move.

I entered the following fix to CvGameUtils.py and it seems to have solved the problem.
Code:
	def AI_unitUpdate(self,argsList):
		pUnit = argsList[0]
		# Absinthe: start Inquisitor AI from Charlemagne (based on SoI)
		iOwner = pUnit.getOwner()
		if not gc.getPlayer(iOwner).isHuman():
			if pUnit.getUnitType() == xml.iProsecutor:
				[B]return[/B] self.doInquisitorCore_AI(pUnit)
				[B][S]return True[/S][/B]
		# Absinthe: end
		return False


	# Absinthe: start Inquisitor AI proper (based on SoI)
	def doInquisitorCore_AI(self, pUnit):

		iOwner = pUnit.getOwner()
		iStateReligion = gc.getPlayer(iOwner).getStateReligion()
		iTolerance = con.tReligiousTolerance[iOwner]
		apCityList = PyPlayer(iOwner).getCityList()

		# Looks to see if the AI controls a city with a religion that is not the State Religion, not a Holy City, and do not has religious wonders in it
		for iReligion in con.tPersecutionOrder[iStateReligion]:
			for pCity in apCityList:
				if pCity.GetCy().isHasReligion(iReligion) and not pCity.GetCy().isHolyCityByType(iReligion):

					bWonder = false
					for iBuilding in xrange(gc.getNumBuildingInfos()):
						if pCity.GetCy().getNumRealBuilding(iBuilding):
							BuildingInfo = gc.getBuildingInfo(iBuilding)
							if BuildingInfo.getPrereqReligion() == iReligion:
								if isWorldWonderClass(BuildingInfo.getBuildingClassType()) or isNationalWonderClass(BuildingInfo.getBuildingClassType()):
									bWonder = true
									break

					if bWonder: continue # skip the code below, and continue with the next city in the loop

					bFound = False
					# If the civ's tolerance > 70 it won't purge any religions
					# If > 50 (but < 70) it will only purge Islam with a Christian State Religion, and all Christian Religions with Islam as State Religion
					# If > 30 (but < 50) it will also purge Judaism
					# If < 30 it will purge all but the State Religion (so the other 2 Christian Religions as well)
					if iTolerance < 70:
						if iReligion == xml.iIslam: bFound = True
						elif iReligion == xml.iCatholicism or iReligion == xml.iOrthodoxy or iReligion == xml.iProtestantism:
							if iStateReligion != xml.iCatholicism and iStateReligion != xml.iOrthodoxy and iStateReligion != xml.iProtestantism:
								bFound = True
					if iTolerance < 50:
						if iReligion == xml.iJudaism: bFound = True
					if iTolerance < 30:
						bFound = True

					if bFound:
						city = pCity.GetCy()
						if pUnit.generatePath(city.plot(), 0, False, None):
							self.doInquisitorMove(pUnit, city)
							return [B]True[/B]
		[B]return False[/B]
If it fails to find a city it should move to, it will return False, in which case the DLL code will make it wait a turn (like hitting space). From a gameplay perspective the result is the same, that is the unit does absolutely nothing. However it figures out that it will not use the unit that turn after looping the cities once, not after looping all cities 100 times. I didn't measure, but the performance impact here must be significant. Looping once is likely slower than the average unit loop and looping 100 times when most units only loop once really mean prosecutors must take up a significant amount of CPU time.

As good as it is to make the game faster, it seems completely unrelated to crashes.

I'm not sure about the modded code in the DLL, which makes prosecutors wait unless they find a specific city to go to. It might be a good idea to make it run to safety if needed or similar, but that is more like a design issue than a bug. This too is completely unrelated to game stability.
 
Wow, Awesome news! :)
Just to be sure, the assert which indicated the bug was this one, right?
Assert Failed

File: C:\Dev\CvGameCoreDLL_RFCE\CvGameCoreDLL\.\CvSelectionGroupAI.cpp
Line: 183
Expression: false
Message:

About your comments:
As good as it is to make the game faster, it seems completely unrelated to crashes.
Yeah, I'm also sure this is unrelated.
Still a huge improvement, it's great that you are this helpful!

I'm not sure about the modded code in the DLL, which makes prosecutors wait unless they find a specific city to go to. It might be a good idea to make it run to safety if needed or similar, but that is more like a design issue than a bug. This too is completely unrelated to game stability.

Indeed, the AI should be improved significantly.
It's even on my Todo list, never got around it though up to this point.
 
I now have 4 savegames, but I can only load the first one. The rest seems to be of a different savegame revision and they crash while loading CvPlayer data. More specifically it seems to be an issue regarding loading plotgroups.

Yeah, I get some 'Assertion Failed' error messages too on loading some of the latest saves.
They might be from revision 1193 - that's exactly the same version as the RFCE 1.3.1 release, so maybe some users are using that one istead of the latest version of 1194.

Also I can continue those savegames, with skipping those initial assert failures.
If it's the same for you too (something about the getID() or some ID_MASK < m_iCurrentID, in line 367 in an array.h file), then just click on pass for a couple times (30+ though!)
EDIT: Maybe I'm entirely off with this, and it's absolutely irrelevant of course, but IMO it's somehow related to the number of players. Also makes sense if we take into account that you mentioned loading CvPlayer data. Thus the number of skips you have to make is iNumPlayers-1 probably, so 32 in the mod (or in one of the saves the double of that for some reason)

Anyway it's unrelated to the CTD, I'm sure it's only because version differences.
I can produce the CTD it in all saves after running them this way.

EDIT: There was another new save around the same time you posted. Couple minutes difference.
http://forums.civfanatics.com/showpost.php?p=14234882&postcount=638
I can run it without those failed asserts, probably that one is from rev 1194.
Did you try that one too?
 
I must have an extremely durable install of civ4. I still can't get it to crash regardless of what I do :confused:

The kind of errors I get on load makes me stop. They are a clear indication that what I read isn't what was written. Even if I can get the game to load, I would end up with a buggy savegame because something went wrong with the samegame format.
 
Back
Top Bottom