Time |
Nick |
Message |
00:13 |
MTDiscord |
<paradust> sfan5: ouch. i can take a look in a bit |
03:51 |
MTDiscord |
<josiah_wi> @savilli I'll plan to check out your PR tomorrow to confirm or deny that it fixes the equality precision rubbish. |
03:51 |
MTDiscord |
<josiah_wi> It's a little late here for me to do it tonight. |
03:55 |
MTDiscord |
<savilli> I've checked it already pretty much, but ok |
04:06 |
MTDiscord |
<josiah_wi> That may be good enough, then. I thought you might appreciate feedback since I have the MSVC build set up. If you have reproduced it as well then I won't need to check. |
04:06 |
MTDiscord |
<josiah_wi> Thank you so much for helping look into this and fix it. |
04:34 |
MTDiscord |
<savilli> Yes, I reproduced it. morelights_chain_ceiling.objwas a good hint. |
05:00 |
|
MTDiscord joined #minetest-dev |
07:52 |
|
calcul0n_ joined #minetest-dev |
07:59 |
|
YuGiOhJCJ joined #minetest-dev |
08:35 |
|
SpaceManiac joined #minetest-dev |
09:38 |
|
Swift110-mobile joined #minetest-dev |
10:22 |
|
[MTMatrix] joined #minetest-dev |
10:43 |
|
fgaz_ joined #minetest-dev |
10:47 |
sfan5 |
@paradust https://0x0.st/Hgad.png |
10:58 |
sfan5 |
0x2c0(%rcx) should if I'm not mistaken be m_dummy_stream |
10:58 |
sfan5 |
but it's merely a member variable, how could that be invalid? |
11:18 |
|
Krock joined #minetest-dev |
11:18 |
|
fgaz__ joined #minetest-dev |
11:18 |
|
Swift110-mobile_ joined #minetest-dev |
11:20 |
|
fluxionary joined #minetest-dev |
11:21 |
|
SpaceManiac joined #minetest-dev |
11:26 |
|
appguru joined #minetest-dev |
12:06 |
MTDiscord |
<wsor4035> @paradust |
12:24 |
|
YuGiOhJCJ joined #minetest-dev |
12:25 |
sfan5 |
http://sprunge.us/xhYbGe?diff doesn't crash with this patch, weird |
12:28 |
|
YuGiOhJCJ joined #minetest-dev |
12:46 |
|
Pexin joined #minetest-dev |
13:03 |
|
Desour joined #minetest-dev |
13:03 |
nrz |
sfan5, order to removal, as m_buffer is in the stream ? |
13:22 |
sfan5 |
could be |
17:38 |
sfan5 |
merging #14054, #14137, #14139, #14141 in 10 or 15m |
17:38 |
ShadowBot |
https://github.com/minetest/minetest/issues/14054 -- Manually configurable minimum protocol version by Warr1024 |
17:38 |
ShadowBot |
https://github.com/minetest/minetest/issues/14137 -- Fix on_(grant|revoke) not being run by mods by appgurueu |
17:38 |
ShadowBot |
https://github.com/minetest/minetest/issues/14139 -- Split windows from linux CI workflows by sfan5 |
17:38 |
ShadowBot |
https://github.com/minetest/minetest/issues/14141 -- Fix set_bone_position regressions by appgurueu |
17:44 |
[MTMatrix] |
<localhost> rolling minetest dev |
17:55 |
|
lhofhansl joined #minetest-dev |
17:59 |
lhofhansl |
Re: log destruction. Looks like bug. According the C++ standard member variables are destroyed in reverse order they appear in the class definition. |
17:59 |
lhofhansl |
sfan5: if your fix works we can use that. |
18:04 |
sfan5 |
code bug or compiler bug you mean? |
18:08 |
lhofhansl |
compiler bug |
18:09 |
lhofhansl |
If members are destructed in reverse order I see no way that log has a problem. |
18:09 |
lhofhansl |
(Or should have a problem) |
18:10 |
lhofhansl |
Maybe the segment heap messes with that order...? |
18:11 |
lhofhansl |
I guess one thing one could try is moving m_stream and m_dummy_stream to the end. That *should* have the same logical effect as your change. But I doubt that's going to help. |
18:12 |
sfan5 |
could also be some weird bug involving large structs in thread_local storage |
18:12 |
sfan5 |
and/or how mingw interacts with the heap |
18:12 |
sfan5 |
I'll wait for @paradust's opinion |
18:14 |
sfan5 |
(note that the order in my patch is actually less safe, the StreamProxys will have dangling pointer momentarily) |
18:14 |
Krock |
how did you get this backtrace? wine-gdb ? |
18:14 |
sfan5 |
gdb on real windows |
18:14 |
Krock |
til that's a thing |
18:14 |
sfan5 |
with a cross-compiled RelWithDebInfo build |
18:16 |
lhofhansl |
sfan5: true (that it's actually less safe) |
18:20 |
Krock |
valgrind might be interesting - if that exists on Windows |
18:24 |
lhofhansl |
While I have your attention... Any further opinion on #13881? I think this is ready. In future might add an extra API to allow a mod to force the tint (instead of calculating it from Rayleigh based on conditions on earth) |
18:24 |
ShadowBot |
https://github.com/minetest/minetest/issues/13881 -- Continuation of x2048's implementation of 'Godrays' by lhofhansl |
18:25 |
lhofhansl |
Would squash it into two commits to leave x2048's original change in place. |
18:30 |
sfan5 |
I am neutral on the feature itself, can give the code a quick look later |
18:30 |
Krock |
2 commits are fine if you're only asking for that |
18:34 |
MTDiscord |
<paradust> I'm not getting a crash with latest windows build (Release, x86, VS 17 2022, commit cad8e895f24c). Is there something I need to do to triggerit? |
18:35 |
|
grorp joined #minetest-dev |
18:41 |
lhofhansl |
paradust: Nope. Just running it and exiting from a game is enough for me to cause this reliably. |
18:42 |
lhofhansl |
Which version of Windows? Segment heap will be ignored on older versions. |
18:42 |
MTDiscord |
<paradust> Windows 10 |
18:43 |
MTDiscord |
<paradust> does it need to be built using Mingw64? |
18:43 |
lhofhansl |
I used the Mingw64 build indeed. I found the msvc builds to be *far* slower (in terms of FPS) |
18:44 |
MTDiscord |
<paradust> is Windows 10 new enough? |
18:44 |
lhofhansl |
Later versions of Windows 10... Lemme check. |
18:48 |
lhofhansl |
Looks like Windows 10, version 2004 (build 19041) and later is required |
18:49 |
lhofhansl |
And version 2004 was around May/June 2020. |
18:51 |
MTDiscord |
<paradust> i have 19045.3803. i'll try to build it up mingw |
18:52 |
MTDiscord |
<paradust> *in |
18:54 |
lhofhansl |
You can just download a build from github... That's what I am doing. Click the "Actions" tab, pick the "build" action, then pick a build (pick the latest on the master branch) and download the mingw64 build from there. |
19:15 |
sfan5 |
19044.3803 here; open minetest, enter world, back to main menu -> unexpectedly closed |
19:15 |
sfan5 |
with the mingw64 build from github |
19:17 |
sfan5 |
ah oops I downloaded win32 by accident |
19:17 |
sfan5 |
but shouldn't make a difference |
19:21 |
sfan5 |
it wouldn't be the first time we hit a mingw bug |
19:38 |
MTDiscord |
<bla8722> win10(64) 19045.3803 here and mine doesnt crash on "back to main menu", neither ming64 artifact nor msvc self compiled |
19:41 |
sfan5 |
ok wow this is awful: if you search for a setting that isn't shown because it depends on something else the settings tab will show zero results |
19:41 |
sfan5 |
in this case i *know* the setting existed and was going insane |
19:54 |
MTDiscord |
<paradust> ok well, I built it mingw-w64-ucrt-x86_64 under msys2, and no crash at all |
19:54 |
MTDiscord |
<paradust> guess I'll try the pre-built one. Could it be a 32-bit only thing? |
20:00 |
sfan5 |
nope it surely happened on 64-bit |
20:01 |
sfan5 |
different gcc versions/runtime? dunno |
20:04 |
MTDiscord |
<paradust> The mingw64 version from https://github.com/minetest/minetest/actions/runs/7291562648#artifacts works fine for me. No crashing |
20:05 |
MTDiscord |
<paradust> the win32 version crashes the moment I run it o_O |
20:06 |
MTDiscord |
<paradust> could there be some difference between 19044.3803 and 19045.3803? Bla says his doesn't crash either, and we're using the same version |
20:08 |
MTDiscord |
<paradust> @bla8722 have you tried the ming32 artifact? |
20:09 |
MTDiscord |
<paradust> oh, the 32-bit crash looks like the GL calling convention bug. It is using an old irrlicht build I assume? |
20:13 |
MTDiscord |
<bla8722> not yet but will try all 4 artifacts of the commit you linked in a few mins |
20:15 |
MTDiscord |
<paradust> oh wait, after several tries it did crash |
20:26 |
MTDiscord |
<paradust> It might be this: https://github.com/msys2/MINGW-packages/issues/2519 |
20:35 |
MTDiscord |
<bla8722> for me both 32bit crash on starting minetest, mingw64 crashes on "back to main menu", msvc64 doesn´t crash. same conf & mtg world for all 4 |
20:36 |
MTDiscord |
<bla8722> no error messages at all or in debug.txt |
20:57 |
MTDiscord |
<paradust> So basically, for thread_local storage, the code that runs destructors and the actual freeing of the TLS memory block happen in different callbacks, and so the ordering between them is undefined. It's possible for the C++ destructor to be called after the memory is already gone. |
20:57 |
MTDiscord |
<paradust> And since this hasn't been fixed in 6 years, it's unlikely to be fixed any time soon. |
20:58 |
MTDiscord |
<paradust> i'll look into workarounds |
21:32 |
sfan5 |
honestly not sure if that's applicable anymore |
21:32 |
sfan5 |
maybe in a different variant |
21:32 |
sfan5 |
e.g. we had https://github.com/minetest/minetest/issues/12022 but that's definitely no more |
21:41 |
MTDiscord |
<paradust> in gdb I'm seeing "this" pointing to an address that is unaccessible |
21:41 |
MTDiscord |
<paradust> that's consistent with it being deallocated before the destructor call.. |
21:41 |
MTDiscord |
<paradust> but I guess it is possible that the pointer was corrupted somehow |
21:42 |
MTDiscord |
<paradust> I'll see if I can rule that out |
21:45 |
MTDiscord |
<paradust> hmm |
22:27 |
MTDiscord |
<paradust> it seems to work 99% of the time when a thread exits, but it is crashing in one particular situation. |
22:28 |
|
lhofhansl joined #minetest-dev |
22:28 |
MTDiscord |
<paradust> Maybe reloaded to unloading a particular library. Maybe there's a way to prevent that thread from using the log system in the first place. |
22:29 |
lhofhansl |
Do other folks also find that the msvc builds are far slower than mingw? (as in 100fps vs 20fps)? |
22:42 |
sfan5 |
a perf regression with msvc was recently fixed fyi |
22:43 |
lhofhansl |
Lemme try again. Although I did last week. |
22:43 |
MTDiscord |
<jordan4ibanez> I always did, would run the goal fps at double the refresh rate. Good to hear that's fixed |
22:50 |
sfan5 |
@paradust looking at it again it seems like yes, this bug in mingw/gcc still exists |
22:50 |
sfan5 |
question is: why couldn't they fix it in 6 years? |
22:53 |
MTDiscord |
<paradust> Maybe they did, but this is an edge case that still exists. Because it working almost always. It's just one thread (which doesn't look like the others... very different address) which is causing crash. Maybe there's something that can be done to prevent it from using the log in the first place |
22:53 |
MTDiscord |
<paradust> If this thread is being created by an external DLL in a strange way, that might explain it |
22:54 |
MTDiscord |
<paradust> trying to figure out what this thread is doing before it crashes |
23:04 |
|
loggingbot_ joined #minetest-dev |
23:04 |
|
Topic for #minetest-dev is now Minetest core development and maintenance. Chit-chat goes to #minetest. https://dev.minetest.net/ https://irc.minetest.net/ https://github.com/minetest |
23:27 |
|
panwolfram joined #minetest-dev |
23:30 |
|
Wuzzy joined #minetest-dev |
23:32 |
|
panwolfram joined #minetest-dev |
23:42 |
MTDiscord |
<paradust> ok I was wrong. It is just happening on a random thread each time |
23:46 |
lhofhansl |
I take it back on msvc, just tried again and it show roughly the same performance mingw. It also does not crash on shutdown. |
23:46 |
lhofhansl |
Why do we have both builds? |
23:50 |
lhofhansl |
Going to merge #13881 in a few (after squashing to two commit) |
23:50 |
ShadowBot |
https://github.com/minetest/minetest/issues/13881 -- Continuation of x2048's implementation of 'Godrays' by lhofhansl |
23:51 |
|
fluxionary joined #minetest-dev |