Dictionary keys pointer problem at runtime: "Invalid handle"

Started by Monsieur OUXX, Sat 08/05/2021 22:05:24

Previous topic - Next topic

Monsieur OUXX

AGS 3.5.0.24. Same issue with 3.5.1.4 (beta 5) and 3.6.0.2

I've never experienced that issue before in any of the thousands of lines of code that I've written.
Annoyingly, I didn't manage to isolate this issue in a simple piece of code. All I can do is provide the complicated AGS project (DOWNLOAD HERE).
I think the issue is in AGS' virtual machine, not in my code. A bug in the stack/heap or something.

Simply open it and run it. You will get "Error releasing pointer: Invalid handle".

This happens on the line containing a closing curly bracket ( } ) because I'm guessing that's where the engine frees local variables.
The issue is definitely with variable "keys" which contains the result of Dictionary.GetKeysAsArrays (i.e. a String[]), and here is how I know it : If you get rid of variable "memberName" and use "keys[ i]" instead directly, then you will get the same error ("invalid handle") on the line where you try to read keys[ i].

Interestingly the error does not happen in any of the subfunctions that have their own local versions of .GetKeysAsArrays (e.g. function ToConsole). It happens only in this block. So I suspect a weird obscure bug in the virtual machine.
I suspect that the handle to the pointer (I mean the handle that's local to this specific code block) becomes messed up when we call GetKeysAsArrays again on the same Dictionary but in another context (inside a subfunction or something) and then try to use the original handle. But it's just a theory.

I'll try with the new compiler, maybe it will help.
EDIT: I don't know how to get a built version of AGS 4.

Here I've rewritten the function without all the obscure code and macros, it's much easier to read:
Code: ags

void MyFunction()
{
        Dictionary* o = ...
        ...

        String keys[] = o.GetKeysAsArray();
        for (int i=0; i<o.ItemCount; i++) {
          //+DEBUG
          o.ToConsole(); //This uses GetKeysAsArray to iterate on every key and write the vale into a file
          //-DEBUG
          
          //AGS engine is having issues managing keys[i] all the way through. At some point it throws "invalid handle"
          String memberName = keys[i];
          
          String memberValue = o.Get(memberName); //You can try to use keys[i] instead of memberName and see what happens
              
          if (memberValue != "undefined") {
            
              ...
              o.Set(memberName, "undefined"); //You can try to use keys[i] instead of memberName and see what happens
              ...
          }
        }
        
       ...
        
      }
}
 

Monsieur OUXX

#1
Bump.
I've tried really hard to find what's wrong. I failed. It's really the engine that, at some point, "loses" the meaning/pointer/handle of keys[ i].

If I do this just before using keys[ i]...
Code: ags
String keys2[] = innerObj.GetKeysAsArray();

... then I can use keys2[2] (no crash) but I cannot use keys[2] (crash).

My immediate suspicion is that it was me who somehow messed with array keys while iterating on it (the single biggest no no in programming, as you know), but if that's the case then I couldn't find where!
 

Crimson Wizard

#2
I managed to pinpoint a line after which the memory breaks. It is "String memberName = keys[ i ];"

Basically, following "works" (as in runs):
Code: ags

String keys[] = innerObj.GetKeysAsArray();
        for (int i=0; i<innerObj.ItemCount; i++) {
					
          //+DEBUG
          innerObj.ToConsole();
          //-DEBUG
}


But this already breaks:
Code: ags

String keys[] = innerObj.GetKeysAsArray();
        for (int i=0; i<innerObj.ItemCount; i++) {
					
          //+DEBUG
          innerObj.ToConsole();
          //-DEBUG

          String memberName = keys[i];
}

Crimson Wizard

#3
So, basically, this causes a crash:
Code: ags

        Dictionary *d = Dictionary.Create();
	d.Set("a", "b");
	{
		String keys[] = d.GetKeysAsArray();
		for (int i = 0; i < d.ItemCount; i++) {
			String memberName = keys[i];
		}
	} // crashes here


It could be that the Strings in that array do have wrong reference count when they are created, so when the reference copy is deleted the string gets deleted already.

Crimson Wizard

#4
Made a fix, will be added to 3.5.1.

The temp build may be found here after the server finishes working:
https://cirrus-ci.com/task/6646367276761088



EDIT: I probably should also make another patch for 3.5.0, this error is too serious to leave it unfixed there.


Monsieur OUXX

Nevermind the hyperbole, you're a f*cking hero. This. this is literally why I stick to AGS.

FYI the continuous build crashed so I can't get my hands on the 3.5.1.x patch. And I've converted my files aay from 3.5.0.x so I can't try it with that one. I'll wait for the next 3.5.1.x build -- as long as I know it's fixed I can work on something else until then  :)
 

eri0o

Monsieur OUXX, there's a successful build of 3.5.1 (non-released, this is just a build of a commit, please backup your game before!) here : https://cirrus-ci.com/build/4539306049011712 (it's a later commit but should have that fix too)

Monsieur OUXX

#8
Quote from: eri0o on Tue 11/05/2021 12:52:21
Monsieur OUXX, there's a successful build of 3.5.1 (non-released, this is just a build of a commit, please backup your game before!) here : https://cirrus-ci.com/build/4539306049011712 (it's a later commit but should have that fix too)

Sorry to be daft but where in the screen do I click to download that build of AGS?

EDIT: Got it.

 

Monsieur OUXX

It works!
Perfect.
Back to debugging my own scripts  := ! Thanks again, you two.
 

Crimson Wizard

#10
Quote from: Monsieur OUXX on Tue 11/05/2021 11:44:21
FYI the continuous build crashed so I can't get my hands on the 3.5.1.x patch. And I've converted my files aay from 3.5.0.x so I can't try it with that one. I'll wait for the next 3.5.1.x build -- as long as I know it's fixed I can work on something else until then  :)

Note that one can also convert back to 3.5.0 by removing 1 line from Game.agf, the one that sais "<AttachDataToExe>".


Quote from: Monsieur OUXX on Tue 11/05/2021 13:54:58
Quote from: eri0o on Tue 11/05/2021 12:52:21
Monsieur OUXX, there's a successful build of 3.5.1 (non-released, this is just a build of a commit, please backup your game before!) here : https://cirrus-ci.com/build/4539306049011712 (it's a later commit but should have that fix too)
Sorry to be daft but where in the screen do I click to download that build of AGS?

Imo the better link would be the one that points to "editor_packaging":
https://cirrus-ci.com/task/5050142681202688
But anyway, guess we need to release 3.5.1 again soon with this fix.


Quote from: Monsieur OUXX on Tue 11/05/2021 14:03:02
Back to debugging my own scripts

I'm scared of your scripts (wtf)

SMF spam blocked by CleanTalk