Minetest logo

IRC log for #minetest-dev, 2014-07-30

| Channels | #minetest-dev index | Today | | Google Search | Plaintext

All times shown according to UTC.

Time Nick Message
00:01 luizrpgluiz joined #minetest-dev
00:01 luizrpgluiz left #minetest-dev
00:05 Zeno` I'm more worried about 1527 heh
00:07 RealBadAngel i can see lighting code there
00:08 Zeno` doesn't seem to be related though. I'll remove that line and re-test
00:08 RealBadAngel please do
00:08 VanessaE RealBadAngel: I surmised this is why nore had to insert that extra check in technic to limit how many VM's it calls into use (a good check anyways since it reduces redundancy)
00:08 VanessaE it was also causing a massive memory leak
00:08 RealBadAngel it often happens that bug resides not where you suppose to
00:10 Zeno` well, the problem is still there: http://codepad.org/tbgNIQ83
00:19 RealBadAngel local vm, emin, emax = minetest.get_mapgen_object("voxelmanip")
00:19 RealBadAngel how this line is supposed to work?
00:19 RealBadAngel set 3 variables to one value?
00:20 RealBadAngel or just the 3rd?
00:21 Zeno` https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_mapgen.cpp#L116
00:21 Zeno` all three are pushed onto the Lua stack
00:23 RealBadAngel i dont know who invented that, but for me that looks like not stack but pile of shit
00:23 Zeno` lol
00:24 Zeno` yeah well, that may be so
00:24 RealBadAngel that is against any coding logic
00:24 Zeno` it's convenient... apparently
00:24 Zeno` I don't like it myself, but *shrug*
00:24 RealBadAngel i do read that as def of three variables and 3rd is defined
00:25 RealBadAngel i mean set
00:25 Zeno` that's how I would (normally) read it as well (i.e. in a C-like way)
00:25 RealBadAngel 1st and 2nd are just set local
00:25 Zeno` or, heck, even pascal or modula-2 or normal basic
00:25 Zeno` but that's Lua for ya
00:25 Zeno` I think python does something similar but I've never used python
00:26 RealBadAngel can you do some tests for me while youre at it?
00:26 Zeno` sure
00:26 RealBadAngel add there before another variable
00:26 RealBadAngel and dump them all on call
00:27 Zeno` before vm?
00:27 RealBadAngel yes
00:27 Zeno` like: local test, vm, emin, emax = minetest.get_mapgen_object("voxelmanip") ??
00:27 RealBadAngel yup, and try later on move test in between
00:29 Zeno` ok, it doesn't work when at the start: 10:28:48: ERROR[main]: ERROR: An unhandled exception occurred: ...est/worlds/ram-test/worldmods/voxelleaktest/init.lua:6: attempt to call method 'get_data' (a nil value)
00:31 Zeno` causes an unhandled exception with a extra at the beginning or the end
00:31 Zeno` actually, anywhere
00:33 Zeno` they're being assigned correctly (the return values)
00:34 RealBadAngel if so, then ok
00:34 RealBadAngel but anyway i still call it ugly as hell
00:35 Zeno` and I would agree :) But, alas, it's not the issue :(
00:35 RealBadAngel who knows if its using stacks
00:36 RealBadAngel one byte not taken out while pushed is a diseaster
00:37 RealBadAngel well, its pretending to be a stack
00:37 RealBadAngel so propably the issue is somwhere else
00:41 Zeno` the issue, I suspect, is the Lua garbage collector and https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_mapgen.cpp#L105
00:41 Zeno` never being deleted properly
00:42 RealBadAngel *(void **)(lua_newuserdata(L, sizeof(void *))) = o;
00:42 RealBadAngel love such code :)
00:43 RealBadAngel you have to be twisted to get it and double twisted to write it
00:44 Zeno` although it *appears to be* here: https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_vmanip.cpp#L34
00:45 Zeno` if (!o->is_mapgen_vm) is dodgy as crap though (any random object has a 50/50 chance of that being true due to the cast)
00:45 Zeno` if !?
00:45 Zeno` if not?
00:45 Zeno` err... does that seem right?
00:45 * Zeno` is confused now
00:46 Zeno` why would you delete it only if it's not NULL ?
00:46 Zeno` not true, rather
00:46 * VanessaE pokes hmmmm
00:48 RealBadAngel if !thing
00:48 RealBadAngel means if thing doesnt exists
00:49 RealBadAngel ie is NULL
00:49 RealBadAngel !NULL = true
00:50 Zeno` yeah, but it's a bool
00:50 Zeno` (I made a mistake when I said !NULL... !NULL would be correct(
00:53 hmmmm Zeno`:  indeed, if (!o->is_mapgen_vm) is a typo
00:53 hmmmm that would definitely cause a memory leak.
00:53 hmmmm the casts are valid though
00:53 Zeno` changing it to  if (o->is_mapgen_vm)  {} causes segfault though heheh
00:54 hmmmm what's the cause of the segfault then?
00:54 nore_ joined #minetest-dev
00:56 RealBadAngel its a bool or a pointer then?
00:56 Zeno` double free (delete) I think... I'm still trying to get my head around what's going on
00:56 hmmmm OH
00:56 Zeno` it's a bool
00:56 hmmmm derp
00:56 RealBadAngel it negates o ?
00:56 hmmmm no
00:56 hmmmm shoot
00:56 Zeno` https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_vmanip.h#L38
00:56 hmmmm I am confused now
00:56 hmmmm nevermind, that's not a typo
00:57 hmmmm if it's a mapgen vm, we don't want to delete that object because it's owned by the mapgens, NOT! lua
00:57 hmmmm LuaVoxelManip::is_mapgen_vm is a bool
00:57 Zeno` although valgrind points to https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_mapgen.cpp#L105
00:57 Zeno` as being the source of the leaked mem
00:57 hmmmm we only want to delete VoxelManipulator objects that were allocated, not grabbed as an existing mapgen object
00:58 Zeno` Where is that memory deleted, is the question I guess
00:58 hmmmm okay
00:59 hmmmm that memory is deleted in Map::finishBlockMake()
00:59 Zeno` hmm ok
01:00 hmmmm ahhhh
01:00 Zeno` must be something else then. sorry for the red herring
01:00 hmmmm I see the problem
01:00 hmmmm Map::finishBlockMake deletes the underlying VoxelManipulator object
01:00 hmmmm regardless of creation method, the LuaVoxelManip must be deleted
01:01 hmmmm so it should be actually like
01:01 hmmmm if (!o->is_mapgen_vm) delete o->vm;
01:01 hmmmm delete o;
01:01 hmmmm sorry
01:02 VanessaE hmmmm: would this be the same thing that caused that big memory leak we had to patch over in technic?
01:02 VanessaE (using non-mapgen vm's to force-load blocks)
01:02 hmmmm i really don't know anything about that
01:03 VanessaE hmmmm: this, except line 83 didn't exist before the patch.  https://github.com/minetest-technic/technic/blob/master/technic/machines/switching_station.lua#L82
01:03 hmmmm if it's a mapgen vm you're getting, then o would never be deleted.  if it's your own vm you're creating, the LuaVoxelManip would be deleted but the VoxelManipulator would be leaked
01:03 hmmmm so in either case something is leaking
01:04 Zeno` yes, that seems to be the case, but I'm new to the code base so I'm battling to understand it :)
01:05 hmmmm alright i'll give an overview of what's happening
01:05 Zeno` you have the test script I am using?
01:05 Zeno` test Lua*
01:05 hmmmm no.
01:05 Zeno` https://github.com/minetest/minetest/issues/1527
01:05 hmmmm you can either create a new LuaVoxelManip object in order to do your own loading and whatever
01:06 Zeno` you can delete the set_lighting() and calc_lighting()... it's not related
01:06 hmmmm or you can grab the existing mapgen's voxel manipulator in order to make any changes to the terrain that has been generated
01:06 hmmmm the existing mapgen's voxelmanip is created in initBlockMake, and deleted in finishBlockMake
01:06 hmmmm as a result, the lua GC can't delete that VoxelManipulator
01:07 Zeno` yep
01:07 hmmmm therefore any LuaVoxelManips created as a wrapper around existing VoxelManipulators must ignore deletion of that VM
01:07 hmmmm but the GC must still delete the LuaVoxelManip wrapper object itself
01:07 Zeno` Ok, I understand the use of the bool now
01:08 hmmmm as for user-allocated voxel manipulators (e.g. used outside of on_generated), the VoxelManipulator is allocated within that same wrapper and thus must be destroyed within the GC
01:08 hmmmm so, indeed, changing that line to "if (!o->is_mapgen_vm) delete o->vm; delete o;" would fix this issue
01:08 Zeno` so line 35 should be delete o->vm and then outside of the condition delete o ?
01:08 Zeno` cool
01:09 Zeno` testing
01:09 hmmmm but
01:09 hmmmm there's more to it
01:09 Zeno` oh oh
01:09 hmmmm look at LuaVoxelManip::~LuaVoxelManip()
01:09 Zeno` ugh
01:09 hmmmm so you can see the destruction logic was poorly thought out at the time
01:10 hmmmm the best way to fix this would be to change l_vmanip.cpp:322 to if (is_mapgen_vm) delete vm;
01:10 hmmmm and then remove the conditional on line 34 entirely
01:11 hmmmm to the best of my knowledge, without testing anything at all, this should solve the issue
01:11 hmmmm i can't be arsed to open up my IDE
01:20 Zeno` testing
01:22 Zeno` so, LuaVoxelManip::gc_object() should simply delete o; ?
01:22 Zeno` after the cast of course
01:25 Zeno` ok, that resolves the issue
01:27 Zeno` I've updated the issue with the changes: https://github.com/minetest/minetest/issues/1527
01:27 Zeno` since I can't push
01:29 Zeno` thanks hmmmm
01:29 Zeno` thanks also for the explanation... apart from the bug fix I understand those source files a bit better now :)
01:37 kaeza joined #minetest-dev
02:20 Zeno` hmmmm, https://github.com/minetest/minetest/pull/1529
02:22 SoniEx2 joined #minetest-dev
02:37 hmmmm yep looks good zeno`
02:37 hmmmm oh wait a minute, you don't have committer privileges
02:38 Zeno` no, you have to do it :)
03:06 smoke_fumus joined #minetest-dev
03:16 cheapie joined #minetest-dev
04:04 NakedFury joined #minetest-dev
04:05 Eater4 joined #minetest-dev
04:28 cheapie joined #minetest-dev
05:41 cheapie joined #minetest-dev
05:45 Hunterz joined #minetest-dev
05:47 grrk-bzzt joined #minetest-dev
05:54 Miner_48er joined #minetest-dev
06:11 kaeza joined #minetest-dev
06:44 Krock joined #minetest-dev
06:51 Kalista joined #minetest-dev
06:56 proller joined #minetest-dev
08:26 Taoki[mobile] joined #minetest-dev
08:43 Kalista joined #minetest-dev
09:22 Calinou joined #minetest-dev
09:41 harrison_ joined #minetest-dev
09:45 SmugLeaf joined #minetest-dev
09:45 SmugLeaf joined #minetest-dev
09:45 blaise joined #minetest-dev
09:46 restcoser joined #minetest-dev
10:18 PenguinDad joined #minetest-dev
10:28 smoke_fumus joined #minetest-dev
10:37 proller joined #minetest-dev
10:50 proller joined #minetest-dev
11:03 kahrl joined #minetest-dev
11:03 kahrl RealBadAngel: wtf is wrong with multiple return values?
11:04 kahrl it's a completely documented feature of lua and a lot of other languages
11:04 kahrl even string.find uses it
11:04 RealBadAngel kahrl, well, nothing in fact. i just dont like how it looks
11:04 RealBadAngel if it works, then ok
11:04 kahrl ah, ok
11:15 ImQ009 joined #minetest-dev
11:22 CheapSeth joined #minetest-dev
11:22 CheapSeth left #minetest-dev
11:22 Zeno` for a language like Lua, multiple return values /is/ the neatest solution
11:23 Zeno` the problem is that local a, b, c = func()   *looks* like c is assigned and not a and b
11:24 Zeno` the only other solution would be to pass a table to the func, kind of like passing a struct in C, but due to Lua's stack mechanism that wouldn't be very efficient
11:25 Zeno` personally I'd like to see some kind of, I dunno, wrapping. Maybe (a, b, c) = func()
11:25 Zeno` but *shrug*
11:25 Zeno` it's not going to change heh
11:25 OldCoder joined #minetest-dev
11:25 Zefram_Fysh returning a table is a nice solution when performance isn't tight
11:26 Zefram_Fysh parenthesised var list would have been a better syntax, but too late now.  you've just got to rely on people knowing the language
11:26 Zeno` Zefram_Fysh, yes, or returning a table (same as passing a table, the performance is the same... not like C where you can pass a pointer to a struct)
11:26 Zeno` yep, that's what I mean by "it's not gonna change"
11:26 Zeno` because it's too late :)
11:29 Zefram_Fysh not too late if you're willing to implement your preferred language in lua.  you can implement it to compile to lua code, which you evaluate with loadstring().  with LuaJIT, this will even have non-horrendous performance
11:30 Zeno` people are used to it now though
11:31 Zeno` and I don't really care enough to fork Lua heh
11:31 Zeno` it's not so bad
11:32 Zefram_Fysh if I were implementing a better language, it wouldn't be just a slightly-modified lua
11:40 Zeno` lua is pretty nice actually
11:40 Zeno` I hated it at first
11:40 Zefram_Fysh it's inoffensive
11:41 Zeno` I began to like it much more when I actually started writing C "extensions" heh
11:41 Zeno` the design it not without flaws, but it's pretty darn good
11:42 Zeno` I couldn't do better
11:42 Zeno` well, maybe I could but only in hindsight
11:47 Zefram_Fysh the 1-offset for numeric indices is obnoxious
11:47 Zefram_Fysh so is the implicit semicolon
11:48 Zefram_Fysh and the implicit creation of variables on typo
11:48 Zefram_Fysh there are many details that are more or less annoying
11:48 Zefram_Fysh but it gets the big things generally right
11:49 Zeno` not related to the language but the common practice of always putting a , after table entries annoys me for some reason
11:49 Zefram_Fysh it's nice to have lambda expressions just work.  you wouldn't have got that in a corresponding language from 20 years ago
11:49 Zeno` lots of languages create implicit vars from typos
11:50 Zeno` well, most "basic like" ones
11:50 Zefram_Fysh putting a comma after the last entry of a table means that adding another entry after it doesn't edit that line, which makes editing easier and the diff less cluttered.  that the language permits it is a desirable feature
11:51 Zeno` Yes, it seems to be desirable to most people. I don't like it though :)
11:51 Zeno` just one of those things I guess
11:51 Zefram_Fysh that many other languages make the same mistake doesn't make it less problematic, nor in this case is it an excuse.  the problems with it are well known, as is the solution
11:51 Zeno` I don't like introducing compound statements in C when they're not necessary either heh
11:52 Zeno` which annoys many other people immensely
11:53 Zeno` I do for () for () for () {}  and people hate me :(
12:31 Jordach joined #minetest-dev
13:17 Calinou joined #minetest-dev
13:29 jp__ joined #minetest-dev
13:31 jp__ left #minetest-dev
13:43 hmmmm joined #minetest-dev
13:46 Eater4 joined #minetest-dev
13:53 Megaf joined #minetest-dev
13:55 AnotherBrick joined #minetest-dev
14:08 NakedFury joined #minetest-dev
14:08 Exio joined #minetest-dev
14:29 harrison joined #minetest-dev
14:30 sfan5 joined #minetest-dev
14:57 swaaws joined #minetest-dev
15:06 werwerwer joined #minetest-dev
15:50 kahrl_ joined #minetest-dev
15:57 Kalista joined #minetest-dev
16:04 Hunterz joined #minetest-dev
16:08 AnotherBrick joined #minetest-dev
16:19 AnotherBrick joined #minetest-dev
16:37 zat joined #minetest-dev
16:47 Calinou joined #minetest-dev
16:51 SoniEx2 joined #minetest-dev
17:42 ImQ009 joined #minetest-dev
18:02 khonkhortisan joined #minetest-dev
18:07 casimir joined #minetest-dev
18:15 EvergreenTree joined #minetest-dev
18:29 cheapie joined #minetest-dev
18:44 Krock hmmmm, I'm almost sure, it has been asked before, but I can't find anything about this. Why does minetest call the function register_on_generated sometimes twice?
18:46 proller joined #minetest-dev
18:54 SmugLeaf joined #minetest-dev
18:54 SmugLeaf joined #minetest-dev
19:05 sapier joined #minetest-dev
19:28 grrk-bzzt joined #minetest-dev
19:28 grrk-bzzt joined #minetest-dev
19:35 hmmmm minetest doesn't call that function, various games do
19:49 vifino joined #minetest-dev
21:15 Megaf_ joined #minetest-dev
21:29 casimir joined #minetest-dev
21:41 Jordach joined #minetest-dev
21:51 smoke_fumus joined #minetest-dev
22:37 blaise joined #minetest-dev
23:21 nore_ joined #minetest-dev
23:34 ^v joined #minetest-dev
23:46 ^v joined #minetest-dev
23:51 kaeza joined #minetest-dev
23:52 diemartin joined #minetest-dev
23:59 Miner_48er joined #minetest-dev

| Channels | #minetest-dev index | Today | | Google Search | Plaintext