[Rseding91] [15.37] Circular references on remote interface = hard crash

[dewiniaid]

The following code in a mod will cause a hard crash (likely stack overflow) in Factorio when called from the console or from another mod

Code: Select all

function boom()
    local a = { b = { } }
    a.b.c = a
    return a
end

remote.add_interface("boom", { boom = boom })

Code: Select all

/c remote.call("boom", "boom")

Discovered by accident while trying to return a much more complicated data structure with children having links to their ancestry in the result.

(Edit: Fixed formatting.)

[Klonan]

Can you post a log file?

[dewiniaid]

Sure. Disabled all mods except my own QuickItemSwap (needed for the eval functionality). Unfortunately I accidentally nuked my freshly created LTN blueprint library in the process -- oops

https://gist.github.com/dewiniaid/7734b ... e07a19be67

Looks like nothing gets logged regarding the crash. That is the entire log file after Factorio crashed, abrupt ending and all. I don't recall if Factorio has any special "The game crashed" sort of handling, but it does not trigger in this case if so (standard Windows application crash messaging instead.)

Exact reproduce here is:

1. Install QuickItemSwap 0.2.0 (exact version posted in case I update the mod in the meantime)
2. Edit QIS's control.lua and remove the comments around api.eval (it seemed dangerous to leave that in production builds!)
3.

Code: Select all

/c remote.call("QuickItemSwap", "eval", "a = { b = {}} ; a.b.c = a; return a")

[quyxkh]

This doesn't reproduce for me in 15.40 plain vanilla:

Code: Select all

    /c remote.add_interface('boom',boom=function() a={b={}} a.b.c=a return a end)
    /c game.print(serpent.line(remote.call('boom','boom').b.c)) -- add/remove `.b.c`'s to see the return's recursive

[Rseding91]

This doesn't reproduce for me in 15.40 plain vanilla:

Code: Select all

    /c remote.add_interface('boom',boom=function() a={b={}} a.b.c=a return a end)
    /c game.print(serpent.line(remote.call('boom','boom').b.c)) -- add/remove `.b.c`'s to see the return's recursive

You're calling a remote interface on the same lua instance which is just a function call when you do it all in the console

[quyxkh]

Ah. And calling a mod-supplied version of that remote certainly does go 'boom', sigsegv on a corrupted doubly-linked list. Wasn't expecting lua to detect that it could shortcircuit the local call, or ... oh, I'm guessing the error's in your copyback? nevermined, y'all are busy enough

[dewiniaid]

Ah. And calling a mod-supplied version of that remote certainly does go 'boom', sigsegv on a corrupted doubly-linked list. Wasn't expecting lua to detect that it could shortcircuit the local call, or ... oh, I'm guessing the error's in your copyback? nevermined, y'all are busy enough

Yeah. The timing was actually kind of funny; I had asked earlier on Discord if it's safe to assume remote.call will always return a copy of the structure (i.e. meaning I don't need to duplicate it myself to prevent other mods from mutating the result) and the answer was "unless you call it from within the same mod".

Oh well. Only cost me 70 lines of code to workaround and the 'safe' data is actually cleaner anyways.

[Rseding91]

Thanks for the report. I've fixed it so in 0.16 you can pass circular references through the remote interface and it will handle it correctly.