Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

stack overflow due to default.dig_up #3075

Open
fluxionary opened this issue Nov 8, 2023 · 2 comments · May be fixed by #3133
Open

stack overflow due to default.dig_up #3075

fluxionary opened this issue Nov 8, 2023 · 2 comments · May be fixed by #3133
Labels
Bug

Comments

@fluxionary
Copy link
Contributor

fluxionary commented Nov 8, 2023 •
edited
Loading 

AsyncErr: Lua: Runtime error from mod '' in callback node_on_dig(): Runtime error from mod '' in callback getAuth(): stack overflow
stack traceback:
	[C]: in function 'read'
	/opt/minetest/bin/../builtin/game/auth.lua:14: in function </opt/minetest/bin/../builtin/game/auth.lua:12>
	[C]: in function 'get_player_privs'
	/opt/minetest/bin/../builtin/game/misc.lua:28: in function 'check_player_privs'
	/opt/minetest/bin/../mods/areas/api.lua:87: in function 'canInteract'
	/opt/minetest/bin/../mods/areas/interact.lua:5: in function 'is_protected'
	/opt/minetest/bin/../builtin/game/item.lua:473: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	/opt/minetest/bin/../builtin/game/item.lua:547: in function 'node_dig'
	...
	/opt/minetest/bin/../builtin/game/item.lua:547: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	/opt/minetest/bin/../builtin/game/item.lua:547: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	/opt/minetest/bin/../builtin/game/item.lua:547: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	/opt/minetest/bin/../builtin/game/item.lua:547: in function </opt/minetest/bin/../builtin/game/item.lua:460>
stack traceback:
	[C]: in function 'get_player_privs'
	/opt/minetest/bin/../builtin/game/misc.lua:28: in function 'check_player_privs'
	/opt/minetest/bin/../mods/areas/api.lua:87: in function 'canInteract'
	/opt/minetest/bin/../mods/areas/interact.lua:5: in function 'is_protected'
	/opt/minetest/bin/../builtin/game/item.lua:473: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	/opt/minetest/bin/../builtin/game/item.lua:547: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	...
	/opt/minetest/bin/../builtin/game/item.lua:547: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	/opt/minetest/bin/../builtin/game/item.lua:547: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	/opt/minetest/bin/../builtin/game/item.lua:547: in function 'node_dig'
	...st/bin/../games/minetest_game/mods/default/functions.lua:301: in function 'dig_up'
	...netest/bin/../games/minetest_game/mods/default/nodes.lua:1416: in function 'after_dig_node'
	/opt/minetest/bin/../builtin/game/item.lua:547: in function </opt/minetest/bin/../builtin/game/item.lua:460>

steps to reproduce:

  1. create a very tall column of papyrus (1024 wasn't enough, 4096 was, worldedit was helpful, but make sure the mapblocks are loaded)
  2. dig the bottom node

this is relatively easy to trigger as an unprivileged player, particularly if ethereal is also installed (papyrus becomes a solid node, and you can nerd-pole on them).

in my local repos, i see it used in cool_trees, ethereal, and naturalbiomes, so the functionality can't be changed without potentially breaking things.

i can't think of a "clean" fix. perhaps, add an additional 4th parameter to default.dig_up, indicating how many nodes it's supposed to dig up. i'll call this n for now for lack of a better name. have this default to something sane like 16 or 128. then, if the method is called and there's "air" for n nodes below the current node, don't continue to dig up - assume we've already dug up n nodes.
@appgurueu appgurueu added the Bug label Nov 8, 2023
@SmallJoker
Copy link
Member

SmallJoker commented Nov 8, 2023 •
edited
Loading

Hacky solution ("duck and cover"):

  1. Assign a new field node.__call_depth in default.dig_up
  2. Modify the original minetest.node_dig to use table.copy(node) instead of the selected field copy
  3. Carry over the node table and increase the call depth counter
  4. Abort default.dig_up if the call stack is too deep

The literal lazy solution:

  1. Offload the task into minetest.after (including player and node validity check)

The "proper" solution for papyrus:

  1. Avoid the recursion, hence avoid default.node_dig. For example:
  2. Find the topmost node
  3. Dig down in a loop. No recursion.

@Emojigit
Copy link
Contributor

Emojigit commented Jun 17, 2024 •
edited
Loading

I would suggest the following solution:

  1. Add a parameter to default.dig_up indicating the maximum height. Above this height, nodes are no longer checked. For papyrus, a value of 10 should be reasonable (naturally they grow to 4, and by hand, it seems insane to put more than that)
  2. Use minetest.get_node() or VoxelManip (I don't know which one is faster) to iterate through nodes above this node. Once another node is encountered, break the loop and record the height.
  3. Dig all the nodes according to the recorded height, from top to bottom. It would be better if we could somehow tell on_dig to skip default.dig_up. (Maybe a mod-scope variable? If that is true the function returns)

@Emojigit Emojigit linked a pull request Jun 17, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Get rid of recursive call in default.dig_up C-C-Minetest-Server/minetest_game
4 participants
@SmallJoker @fluxionary @appgurueu @Emojigit