Skip to content

lume.hotswap - Fix three edge cases that lead to a crash#27

Open
rameshvarun wants to merge 1 commit intorxi:masterfrom
rameshvarun:master
Open

lume.hotswap - Fix three edge cases that lead to a crash#27
rameshvarun wants to merge 1 commit intorxi:masterfrom
rameshvarun:master

Conversation

@rameshvarun
Copy link

@rameshvarun rameshvarun commented Aug 25, 2018

I found three edge cases that can lead to a crash in lurker. All of these have to do with calling update on a non-table object. In order of how reasonable they are:

  1. Adding a new table field to an updated object
return {}
return {a = {}}
  1. Changing a global from a table to a value
a = {}
a = 3
  1. Changing the return value of a module from a table to a value
return {}
return 3

One of these can be solved by just checking in update itself, but the other two require checks before calling update.

if oldmt and newmt then update(oldmt, newmt) end
for k, v in pairs(new) do
if type(v) == "table" then update(old[k], v) else old[k] = v end
if type(v) == "table" and type(old[k]) == "table" then
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If table -> value or value -> table, just do an assignment instead of an update.

package.loaded[modname] = nil
local newmod = require(modname)
if type(oldmod) == "table" then update(oldmod, newmod) end
if type(newmod) == "table" and type(oldmod) == "table" then
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only merge if both oldmod/newmod are both tables. This could be replaced with a check inside update but since the other call paths need to check out of update I thought consistency would be better.

end
for k, v in pairs(oldglobal) do
if v ~= _G[k] and type(v) == "table" then
if v ~= _G[k] and type(v) == "table" and type(_G[k]) == "table" then
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only update + replace with updated value if both old and new are tables.

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

Successfully merging this pull request may close these issues.

1 participant