HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.18k stars 656 forks source link

SPOD problem #3897

Closed mockey closed 9 years ago

mockey commented 9 years ago

I have a rather old web app here targetting neko using SPOD. I just saw that it doesn't compile anymore. With current Haxe dev version, I get a stack overflow in String.hx, caused by some loop in RecordMacros.hx apparently. The last time I got it compiled was in March 2014, so I simply grabbed the RecordMacros.hx from before this commit: cbd46ec353dc5091b77562d59fdf0ecfdadd0aba. This compiles fine. Apparently something seems to have changed (or was messed up?) with the handling of relations in this commit. Any idea? Also IMO things like SPOD should really been taken out of the std-lib and put into separate libraries.

mockey commented 9 years ago

Uhm, the problem seems to be: var relatedInf = getRecordInfos(makeRecord(resolveType(r.type))); getting called inside getRecordInfos here: https://github.com/HaxeFoundation/haxe/blob/development/std/sys/db/RecordMacros.hx#L365 producing an (infinite?) loop with a resulting stack overflow. At least in my app here with 17 relations. I'll try to investigate further...

mockey commented 9 years ago

I don't quite understand what is done there, checking the types of the related keys it seems. As a quick fix I simply added a flag to getRecordInfos and check it before the relations loop and run: var relatedInf = getRecordInfos(makeRecord(resolveType(r.type)), false); inside the loop so that relation types won't check their relations and so on. This compiles for me at least. Not sure if this is a proper fix, though...

ncannasse commented 9 years ago

@mockey could you post a small reproducible sample ? @waneck could you look at it for 3.2 ?

mockey commented 9 years ago

A bit difficult. It needs a running database and all, doesn't it? Do you have some test setup for SPOD?

ncannasse commented 9 years ago

@mockey just a standalone SPOD class that reproduces the issue will be enough

waneck commented 9 years ago

@mockey I've just pushed a possible fix to it. Can you see if it works now?

mockey commented 9 years ago

@waneck Yes, works fine. I thought it might have something to do with caching but didn't know where to put the set call.

waneck commented 9 years ago

Great, then!

ncannasse commented 9 years ago

@waneck you might want to make sure that this does not cause issues with compilation server (with macro context cache)

@delahee CC

waneck commented 9 years ago

I'm not sure what the compilation server would have to do with this problem. The problem was with circular dependencies on SPOD. I think that even with the compilation server, the globals variable would get restartes each compilation, so there would be no problem with that

ncannasse commented 9 years ago

@waneck I meant, that you added a cache of the schemas (in static var maybe ?) and the cache will not be reset between compilations

waneck commented 9 years ago

The cache was already there, and it was you who added it ;)

ncannasse commented 9 years ago

So I have hopefully already did some Context.onMacroContextReused setup ;)