crash in slow timer

[lazedo]

Description

on busy servers, there's a sporadic crash in slow timer.
we don't have any debugging data anymore nor a way to reliably reproduce it.
just posting here in case someone sees the same and can find a way to reliably reproduce it and maybe test the patch below.

Troubleshooting

Reproduction

Possible Solutions

with the patch below we don't see any crashes
either tl or tl->f is null when it crashes. this is probably some race condition

diff --git a/src/core/timer.c b/src/core/timer.c
index 0e0dc8812..3c59db3da 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -1128,7 +1128,7 @@ void slow_timer_main()
 #endif
 				SET_RUNNING_SLOW(tl);
 				UNLOCK_SLOW_TIMER_LIST();
-					ret=tl->f(*ticks, tl, tl->data);
+					ret= (tl->f ? tl->f(*ticks, tl, tl->data) : 0);
 					/* reset the configuration group handles */
 					cfg_reset_all();
 					if (ret==0){

Additional Information

• Kamailio Version - output of kamailio -v

5.1 / 5.2

• Operating System:

CentOS7

[miconda]

Are the latest 5.1 / 5.2 versions? It seems to be some reset timer struct used there.

I am fine to push such safety check, but I would do it with some warning log message. I am going to push a patch and then you can adjust further if you need something else.

[lazedo]

@miconda yes, latest 5.1 / 5.2 under heavy load. thanks

[miconda]

Pushed 574b080 .

Have noticed some blocking of/slow message processing before that happening?

[lazedo]

no warnings , just crash.
what are the odds of tl being null ? we had some crashes where fl was null. maybe something is grabbing or invalidating the timer between UNLOCK_SLOW_TIMER_LIST and next instruction ? is that even possible ?

[miconda]

Did you mean f or tl in the "we had some crashes where fl was null"?

Probably the tl should not be null, because its value (pointer) should be copied in the timer process list. But its content depends on each module that adds items to the timer list. So tl in timer list should be a non-null value, but can end up pointing to an address which is no longer a timer_ln struct.

[lazedo]

sorry, meant to write tl

[miconda]

OK.

Closing this one, with the patch applied -- if you get new data, reopen or create a new issue.