Advanced Question ctd when rapidly deleting multiple docking ports

[jedidia]

I was thinking about posting this in the Orbiter bugs thread, but I can't make enough sense of it to justify that at the moment.

The ctd I'm getting is very weird. Namely, if one of the docks I delete has still another vessel docked to it, or had a vessel docked to it that still exists in the scenario, Orbiter gets a memory exception somewhere in the process (ctd can happen a few frames later, sometimes almost a second).

This is the error message I'm getting from visual studio:

First-chance exception at 0x00423756 in orbiter.exe: 0xC0000005: Access violation reading location 0xfeeefeee.
Unhandled exception at 0x00423756 in orbiter.exe: 0xC0000005: Access violation reading location 0xfeeefeee.

It also happens if I undock all vessels first, and seemingly still significant time after the vessels have been undocked, and even if the scenario has been saved and reloaded, if Orbiter wasn't quit completely in between (closing the launchpad or using re-spawn).

If Orbiter is quit completely, the scenario with the undocked vessel can be loaded and the docking ports deleted safely.

If there are no vessels in the scenario that have been docked to the vessel during this session (either none have ever been docked or they have been deleted before deleting the docking ports), the ports also get deleted safely.

No ctd happens if I delete specific docking ports, even if things are docked to them.

Also, and now it gets even weirder, no ctd if I delete all docking ports. You can see the function I'm using for deletion below. The only dockingport returning true for IsPermanentDockPort is currently dockport 0. If I rig IsPermanentDockPort to always return false (while still doing everything it is doing normally, so it can't be anything in that function either), all docking ports on the vessel get deleted without ctd. If I leave at least one docking port, and the conditions described above are satisfied, I get a ctd.

Does anyone have the faintest idea what could be going on here?

void IMS::FinalizeConstruction()
{
	Undock(ALLDOCKS);    //ctd happens with or without this line

	for (UINT i = 0; i < DockCount();)
	{
		DOCKHANDLE CurDock = GetDockHandle(i);
		if (!IsPermanentDockPort(CurDock)) 
		{
			if (i < cmParams.NumTotDocks - cmParams.DeleteDocks.size()) cmParams.DeleteDocks.push_back(i);
			DelDock(CurDock);
		}
		else ++i;
	}
}

---------- Post added at 05:19 PM ---------- Previous post was at 01:38 PM ----------

Some more info:

Since I don't get a crash when I delete all docking ports, I tried a workaround by deleting all of them, and then re-creating those that have to stay. The result is the same: ctd not quite immediately, but closely following execution. The circumstances under which it occurs seem to be the same as above.

Maybe worth noting, there's a lot of center of gravity shifting going on while the add-on is running. Not during this execution, but since cog-shifts influence docking ports, maybe it's got something to do with it.

Is it possible that there's a memory leak or somesuch in connection with deleting docking ports? as it is not common for Orbiter vessels to frantically spawn and delete docking ports, I could at least imagine something like this having gone unnoticed so far.

---------- Post added 02-12-12 at 03:31 PM ---------- Previous post was 02-11-12 at 05:19 PM ----------

Did some more research and took the issue over to the bug reports. This thread may be closed.

 

[kuddel]

Hello Jedidia,

I am not fully understanding what you like to acomplish, but when looking at that code...

void IMS::FinalizeConstruction()
{
    Undock(ALLDOCKS);    //ctd happens with or without this line

    for (UINT i = 0; i < DockCount();)
    {
        DOCKHANDLE CurDock = GetDockHandle(i);
        if (!IsPermanentDockPort(CurDock)) 
        {
            if (i < cmParams.NumTotDocks - cmParams.DeleteDocks.size()) cmParams.DeleteDocks.push_back(i);
            DelDock(CurDock);
        }
        else ++i;
    }
}

...it gives me the creeps...
Please try to answer these questions yourself, just by looking at the code:
What happens when there's just one Docking Port (DockCount()==1)?
What happens if this fulfills your "if" statement?

You will see, that this is not very easy to understand/explain and therefore this code is a perfect example of code that will bite you sooner or later!

As a (general) rule of thumb: You should almost always increment/decrement the counting valibles without any if/else expression, 'cause that makes code hard to analyze or check!

I don't know how experienced you are with C/C++, but my recommendation comes from many years of coding. Worst parts are those that I've written in my early years...and of course those I see when I had to fix problems in code written by someone else

I am pretty sure (but I might be wrong) that you meant something like:

void IMS::FinalizeConstruction()
{
  for (UINT i = DockCount(); i >= 0; i--)
  {
    DOCKHANDLE CurDock = GetDockHandle(i);
    if (!IsPermanentDockPort(CurDock)) 
    {
      // Only delete when this is true (whatever it means ;) )...
      if (i < cmParams.NumTotDocks - cmParams.DeleteDocks.size()) {
        DelDock(CurDock);
      }
    }
  }
}

First thing: The DockCount() will be called ony ONCE here.
Second I did a count-down loop, 'cause I think it's more save so, else I think we would have to call DelDock() with i=0 several times.

Anyway, if this is not what you meant to do, there are two reasons:
1. I didn't understand (most probable)
2. I couldn't "see" what your code "should" do (writing code that is easy to read and to understand is an art-form itself, though)

Happy coding ,
Kuddel

 

[jedidia]

What happens when there's just one Docking Port (DockCount()==1)?

Loop will execute once.

What happens if this fulfills your "if" statement?

Docking port will be deleted, DockCount will return zero, loop will exit. That's the way it should be.

for (UINT i = DockCount(); i >= 0; i--)

You forget that docking indexes are zero-based, while DockCount is one-based

It's also no problem to have the same index deleted multiple times, because the above indexes will push down (which is why i is only incremented when no docking port was deleted, otherwise I'll miss one).
I must admit though that counting down instead of up would have been the more elegant (certainly the more readable) solution. Didn't occur to me at the time.

Anyways, this thread isn't really current anymore, as written in the last line. I have narrowed causes down, created a minimal code example using the shuttlePB and posted the whole thing here.

It's definitely an Orbiter Bug, but as far as I can see noone has either tried or succeeded to reproduce it so far....

 

[kuddel]

You forget that docking indexes are zero-based, while DockCount is one-based

you're surely right! Damn, I've missed that
I hate myself for posting such a stupid thing.
Hopefully someone else -- trying to learn C++ -- reads both posts (mine and yours).

 

[orb]

You forget that docking indexes are zero-based, while DockCount is one-based

Hopefully someone else -- trying to learn C++ -- reads both posts (mine and yours).

So they can read 3rd post too, and since UINT is always equal or larger than 0, this would be better:

for (int i = DockCount () - 1; i >= 0; --i)

 

[kuddel]

So they can read 3rd post too, and since UINT is always equal or larger than 0, this would be better:

for (int i = DockCount () - 1; i >= 0; --i)

Right, but as DockCount() is declared/defined as "UINT DockCount () const;" this will likely create a compiler warnig about comparing signed and unsigned types.
But the pre-decrement is surely better/faster than the post-decrement in the update-section[1] of your for-loop.
O.K. I think this tends to go slightly off-topic now.

[1] for (<initialization>; <condition>; <update>) { <statement>; }