clugg / sm-json

A pure SourcePawn JSON encoder/decoder.
GNU General Public License v3.0
82 stars 8 forks source link

Exception on recursive JSON decoding #3

Closed EnriqCG closed 5 years ago

EnriqCG commented 5 years ago

Hello,

I'm having issues decoding a JSON that contains nested objects or arrays inside. Frustated with not knowing what the issue is, I copy-pasted the example in the README but fails too, so seems to be a library-issue.

public void onEvent(const char[] json) {
    // function is empty. just testing this
    // copied from README.md
    JSON_Object obj = json_decode("{\"myfloat\":73.570000,\"myobject\":[],\"myhandle\":null,\"mybool\":false,\"name\":\"my class\",\"myint\":9001}");
}

gives the following error on the console

Exception reported: Not enough space on the stack
[SM] Blaming: telemetry.smx
[SM] Call stack trace:
[SM] [1] Line 244, sourcemod\scripting\include\json.inc::json_decode
[SM] [2] Line 304, sourcemod\scripting\include\json.inc::json_decode

Thanks for your time.

clugg commented 5 years ago

Hi Enrique, Thanks for pointing this out. It was the result of a couple of different things.

I failed to mention in the README that due to the large amount of stack space the library uses, you would likely need to increase your plugin's stack space using #pragma dynamic - as you can see in json_test.sp, I had modified the stack space accordingly.

https://github.com/clugg/sm-json/blob/e8c9fca490a5cbfea5ab961b6bc0988d76662f24/addons/sourcemod/scripting/json_test.sp#L37

You can read more about #pragma dynamic here, although unfortunately this is the only information I've found, as I can't find a mention of it anywhere in the sourcemod docs. While adding this to your code would likely fix your issue, I believe that you uncovered a larger issue: I was unnecessarily using more stack space than was actually required. I defined an arbitary JSON_BUFFER_SIZE value which was rather large and used in a few different places.

However, after going through the code again I've found that each instance where that constant was used could be optimised to use a known value instead. As such, I've optimised the library to the point where you should no longer need to modify your stack space using #pragma dynamic, unless you are directly working with larger JSON structures. You can find all of the relevant commits below.

https://github.com/clugg/sm-json/commit/8a579276de65f8aa85ef8622d8633deec3eddbe9 https://github.com/clugg/sm-json/commit/64fa568b3093763e1f647297519d8ae3c5250b33 https://github.com/clugg/sm-json/commit/1781888329123d59d9cad78a160f72ec25993cce https://github.com/clugg/sm-json/commit/b3961a0df262cd5cd61c255f31160d6a74e8a2ee https://github.com/clugg/sm-json/commit/a02ab760f3917a4afa171f650f8d158d976024b4 https://github.com/clugg/sm-json/commit/fdff70f5bb198d67243ac812247b2eacd8eefd47

Please let me know if you run into any more issues, and thanks again!

EnriqCG commented 5 years ago

I can confirm it works without using #pragma dynamic.

Thanks a lot for the descriptive response and the quick fix!