Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mky.mojo2 - Crash in Destroy() method of TMaterial #7

Open
wombatswaffles opened this issue Feb 26, 2024 · 12 comments
Open

mky.mojo2 - Crash in Destroy() method of TMaterial #7

wombatswaffles opened this issue Feb 26, 2024 · 12 comments

Comments

@wombatswaffles
Copy link

When running the rendertoimage.bmx example, it will eventually crash on the line where the image is created. The crash appears to be coming from the Destroy() method of TMaterial.

@GWRon
Copy link
Contributor

GWRon commented Feb 27, 2024

Can you please run it in a debug build and try to catch a "crash" so you can get a kind of backtrace (the debug-panel in maxide shows the "history" of commands and the content of the current objects) ... so maybe try to provide screenshots of them if you think that helps.

(I cannot replicate it for now in my linux mint 21.1 setup).

PS: had to update one line in it to make it compile at all:

				icanvas.SetColor(Float(Sin( MilliSecs()*.1 )*.5+.5), Float(Cos( MilliSecs()*.1 )*.5+.5), .5)

@wombatswaffles
Copy link
Author

Sorry, I should have put more information in my post. I tried it on Windows and it works okay, so it's a bit odd. It doesn't happen immediately on macOS...it takes around ten seconds or so.

The error is, "Unhandled Exception:Attempt to access field or method of Null object".

Unfortunately, the Debug panel doesn't work perfectly on macOS, so I'm not sure if the screenshot I've attached is missing anything.
Untitled 3

@GWRon
Copy link
Contributor

GWRon commented Feb 27, 2024

Seems "nodeenum" is null. Dunno if it tries to access this in the "hasnext" somehow.

Which line does it highlight in the editor?

Maybe @woollybah could check it out on his mac?

@wombatswaffles
Copy link
Author

wombatswaffles commented Feb 27, 2024

In rendertoimage.bmx, it highlights:

Local image:TImage = New TImage.Create( 256,256 )

It highlights the second line of the Destroy() method of TMaterial in graphics.bmx.

@GWRon
Copy link
Contributor

GWRon commented Feb 27, 2024

Maybe something gets deleted while it is accessed via the enumerator?

Eg a texture could be stored in the map multiple times with different keys (param)
When setting a texture the old is fetched, the new is "retained" (increase refcount). Then the old is freed (decreases refcount).

But...if the key/param is "ColorTexture"..then this is assigned to _colorTexture as well (with refcount possibly being 1)

Then when deleting the material all textures stored there are "free()"d .. which also calls Destroy() and so removes gltextures etc.
The refcount reaches 0 ..but _colorTexture still contains a ref to the texture...which is now containing no gltex etc anymore.

Am on my phone...so not that easy to check the code...so maybe behaviour as this leads to some null object somewhere...as soon as something eg goes out of scope and the GC thinks it is time to cleanup memory.

Maybe prepend a GCSuspend() at the start of the file and check if it survives longer then

@wombatswaffles
Copy link
Author

wombatswaffles commented Feb 27, 2024

I reinstalled BlitzMax to make sure I wasn't just using an old version, but the problem remains.

Putting GCSuspend at the start of the example does allow it to survive. I do wonder if something is getting deleted before it should...or not even added when it should? I don't know enough about the garbage collector.

@GWRon
Copy link
Contributor

GWRon commented Feb 27, 2024

in "debug builds" the elements survive "longer" - so sometimes the crashes do not happen there.
Can be vice-versa of course - so stuff exists longer than "expected" (not yet cleaned up ...).

Regarding debug panel: you might have a whirl with "vs code" and the blitzmax extension to have some debugging stuff spit out there.

PS: as you confirmed that a GCSuspend() helps to survive it almost MUST be something cleaning up too much/early.
Alternatively something is not marking something as "to retain" and it is seen as "to cleanup" by the GC.

@GWRon
Copy link
Contributor

GWRon commented Feb 27, 2024

I tried it in macos 12 (in a VM) and was not able to make it crash there ...maybe my VM is too slow to have the required effect.

Edit: If you think TStringMap contains an issue ... (as it happens in the iteration over "_textures") you could replace (in graphics.bmx)

Type TMaterial Extends TRefCounted
...
	Field _textures:TStringMap=New TStringMap

with

Type TMaterial Extends TRefCounted
...
	Field _textures:TMap=New TMap

Maybe something there leads to the enumerator (or properties of it) become collected/inaccesible/null

@wombatswaffles
Copy link
Author

I just tried it on my Intel MacBook Air running 11.3 and it hasn't crashed/errored. The issue happens on my M2 Mac mini, so that's interesting.

@GWRon
Copy link
Contributor

GWRon commented Feb 27, 2024

Another remark:
You initially mentioned Method Destroy() of TMaterial.

checking graphics.bmx:
TMaterial.Destroy() is only called when TMaterial.Free() is executed.
TMaterial.Free() is only called when TImage.Discard() is executed.
TImage.Discard() is only called when TImage.Delete() is executed.

TImage.Delete() is called if the GC cleans up (so it is the deconstructor).

This means something is garbage collecting an TImage-Instance. And the example/sample code does that nowhere.

Might the M2-stuff have issues with the bdwgc-garbage collector or the compiler doing unexpected things to our code @woollybah ?

@GWRon
Copy link
Contributor

GWRon commented Feb 27, 2024

I checked what TImages are created:
one via TImage.Load("incbin::data/mojo2_font.png", ...) and the second is the image in the example.

Might it be that the incbin goes kinda "out of scope" (so it is not that image in the example code but another one) ?

@wombatswaffles - could you check out something?

Add this to graphics.bmx Type TImage

	Method New()
		print "New TImage " + self.ToString()
	End Method

And find Method Delete() and replace it with:

	Method Delete()
		print "Delete TImage " + self.ToString()
		Discard()
	End Method

Output for me is:

Executing:rendertoimage
New TImage 0x7f6963dd8cd0
New TImage 0x7f6963dd8c80

(you will see different memory addresses of course - both will differ to each other)

If something is "officially" deleting an object (eg GC decides it is ripe for cleanup/ nobody using it) then you will also see
Delete TImage ...

And if that is the case you would know "which" image of the two is collected. If it is the first, then the incbin-font-image is cleaned up, if it is the second, then your image created in the example.

@wombatswaffles
Copy link
Author

I get:

New TImage 0x109f3fc80
New TImage 0x109f3fc30

And well, oddly, the problem has stopped happening for now. I guess I'll see how it goes over the next couple of days. Maybe rebuilding the modules fixed something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants