Actium Posted February 16 Posted February 16 Running the the following Lua code will instantly crash DCS (client and dedicated server alike): local t = {} t[1] = t return net.lua2json(t) On the dedicated server I got the following log messages, indicating a segmentation fault (access violating): 2025-02-16 21:21:06.058 INFO EDCORE (Main): try to write dump information 2025-02-16 21:21:08.560 INFO EDCORE (Main): # -------------- 20250216-212108 -------------- 2025-02-16 21:21:08.560 INFO EDCORE (Main): DCS/2.9.12.5336 (x86_64; MT; Windows NT 10.0.18362) 2025-02-16 21:21:08.560 INFO EDCORE (Main): 2025-02-16 21:21:08.560 INFO EDCORE (Main): # C0000005 ACCESS_VIOLATION at 0000000380025f70 00:00000000 2025-02-16 21:21:08.560 INFO EDCORE (Main): SymInit: Symbol-SearchPath: 'C:\DCS_server\bin;', symOptions: 532, UserName: 'dcs' 2025-02-16 21:21:08.560 INFO EDCORE (Main): OS-Version: 10.0.18362 () 0x0-0x1 2025-02-16 21:21:13.483 INFO EDCORE (Main): Minidump created. Presumably, net.lua2json() does not check for cyclic references, recursing infinitely, and finally smashing past the end of the stack, which causes the segfault. 1
Actium Posted February 16 Author Posted February 16 Mission file for easy reproduction. Launch the mission, watch the countdown, see the game window close. Crash_net.lua2json.miz 1
Flappie Posted February 16 Posted February 16 Thank you, @Actium! Issue reproduced and reported. 2 ---
Flappie Posted February 17 Posted February 17 (edited) @Actium Devs are working on a fix to prevent a crash from happening with this wrong usage of the API. I'm pretty sure you were simply testing the limits of this function, but just in case, devs made it clear that tables with circular dependencies inside will never be in any circumstances correctly converted to JSON. Edited February 17 by Flappie 1 ---
Actium Posted February 17 Author Posted February 17 @Flappie: Thank your for the update and prompt action from the devs! Please forgive me for the following jibe: For an API usage to be "wrong", there'd need to be documentation that specifies what is right. However, DCS_ControlAPI.html is quite terse on net.lua2json(): "net.lua2json(value) -> string: Convert a Lua value to JSON string" Obviously, cyclic data cannot be expressed in JSON. Regardless, using an API incorrectly should always result in a recoverable error (message) and never in a segmentation fault. Here's an MIT-licensed implementation for serializing arbitrary Lua values (including those with cycles) to valid JSON without crashing in as little as ~100 lines of code: https://gist.github.com/ActiumDev/4c2a4b9ea1a638d131f1ab38ec61f16f#file-webconsole-lua-L408 (part of my standalone web console) Feel free to use it according to the terms of the MIT license. Just FYI: I'm available for renumerated consulting.
Flappie Posted February 17 Posted February 17 41 minutes ago, Actium said: Regardless, using an API incorrectly should always result in a recoverable error (message) and never in a segmentation fault. Devs agree, which is why DCS will be fixed thanks to your feedback. I don't know if they plan to add an error message in the log, but they often do. ---
Actium Posted February 18 Author Posted February 18 (edited) 21 hours ago, Flappie said: I don't know if they plan to add an error message in the log, but they often do. Just to avoid any possible misunderstanding and on account of the plentiful and – for the sake of backwards compatibility – unfortunately irreversible crutches* in DCS' Lua API, please indulge me for pointing out the following: Adding an error message to the log is the worst possible "fix" for this particular issue, as it cannot be caught programmatically**. Always enable use of Lua's integrated error handling mechanism pcall(), when possible. For net.lua2json(), I would suggest the solutions 1 thru 3 in my order of personal preference: Serialize Lua values that cannot be represented in JSON, e.g., cyclic references, functions, userdata, etc., as null. Neither pairs() nor ipairs() will ever return a nil-valued key or value. When deserializing from JSON back to Lua, these null-values will simply disappear. No harm done, but valuable, human-readable debugging insight added. Simply skip all values that cannot the be serialized to JSON. Same outcome as above when deserializing, but less debugging friendly. Inconvenient: Use Lua's error() or lua_error() functions, respectively, to throw an exception that can be handled somewhere up the chain. Would make net.lua2json() useless for some types of data, but we can implement suitable alternatives. DO NOT: Return a string with an error message* instead of a string containing the serialized data. A huge footgun and inconsistent API design as it facilitates passing along an error message instead of valid JSON without noticing. Feel free to DM me, if you or your Devs have any questions. *) I'm pointing this out explicitly, because net.dostring_in() does exactly that, and, as a cherry on top, it has a second boolean return value that indicates success/failure and thus whether the first return value is the actual string return value or a string error message. Of course, the second return value is undocumented. Appears inspired by pcall(), although the order of return values got messed up. Regardless, only pcall() needs to return success/failure as a separate boolean return value. All other functions can simply defer their error handling to the proper use of pcall(). UPDATE: And as another undocumented "feature" and unreasonable behavior, net.dostring_in() will simply return nil when called from a client connected to a server **) For the sake of proving that I did my homework, it could be done with DCS.getLogHistory(), but that would be a truly ridonculous crutch. Edited February 18 by Actium
Solution Actium Posted March 20 Author Solution Posted March 20 (edited) This issue has been fixed in DCS 2.9.14.8222. Unserializable values will now be substituted with null. The following example code local t = {a = 1, b = 2, d = 4} t["c"] = t return net.lua2json(t) now returns {"a":1,"d":4,"c":null,"b":2}. Edited March 20 by Actium 1
Recommended Posts