raduprv / Eternal-Lands

http://www.eternal-lands.com
Other
158 stars 57 forks source link

crash while suddenly rotating the camera in invance. #45

Closed ht990332 closed 6 years ago

ht990332 commented 6 years ago

I got a crash while rotating the camera during an invance. this was at the beginning with lots of monsters, people, and also probably a lot of fighting/magic effects. I'm attaching a gdb backtrace captured with coredumpctl. gdb.txt

pjbroad commented 6 years ago

Looks like a fault in in the eye candy code raised an exception. The EC code has changed very little for many years so its probably been around for some time. This will take some digging.

#4  0x00005582f213a4e8 in ec::Particle::draw (this=0x5582f72ce7c0, usec=16645)
    at eye_candy/eye_candy.cpp:1139
        burn = 1
        tempalpha = 0.387206018
        tempsize = nan(0x400000)
        __PRETTY_FUNCTION__ = "void ec::Particle::draw(Uint64)"
        texture = 21890
ht990332 commented 6 years ago

I rebuilt with debug SDL. I'll check if I get a crash again.

pjbroad commented 6 years ago

I've had a quick look. There is any easy way to detect the conditions of the assertion and do something sensible rather than force and exit, but I've not yet tracked down the route cause. The same function has several other assertions. We should perhaps catch all of these, log the errors then continue, rather than exit. While I've done it myself, we should not really leave assertions in release code... https://www.softwariness.com/articles/assertions-in-cpp/

pjbroad commented 6 years ago

This change should prevent the assertion.

diff --git a/eye_candy/eye_candy.cpp b/eye_candy/eye_candy.cpp
index 408f5c75..b33d994b 100644
--- a/eye_candy/eye_candy.cpp
+++ b/eye_candy/eye_candy.cpp
@@ -1201,7 +1201,9 @@ namespace ec

                const coord_t exp = std::pow(exp_base, flare_exp);
                const coord_t flare_val = 1.0 / (exp + 0.00001);
-               if (flare_val > flare_max)
+               if (!std::isfinite(flare_val))
+                       return 1.0;
+               else if (flare_val > flare_max)
                        return flare_max;
                else
                        return flare_val;
ht990332 commented 6 years ago

Yes, I learned those are normally only in debug builds when I last compiled debug webkitgtk+ and found it unusable.

I'll try the patch. Thank you very much.

pjbroad commented 6 years ago

Any more crashes? I may commit the change anyway as it can only do good....

ht990332 commented 6 years ago

I haven't seen any crashes since I applied the patch.

pjbroad commented 6 years ago

Closing as should be handled safely now.