Fix serialization of LLTimers and LLEvents, add test

Author: HaroldCindy

VM/src/ares.cpp

@@ -1401,68 +1401,64 @@ static void u_userdata(Info *info) {                                   /* ... */
       }
       case UTAG_LLEVENTS:
       {
-          // Because we have an inner, wrapped table reference we need to reserve
-          // the idx for the outer event handler first, since we saw it first.
-          int reference = allocate_ref_idx(info);
-
-          unpersist(info);                                /* ... handlers_tab */
-          eris_checktype(info, -1, LUA_TTABLE);
-          // We need to add a ref to the table so it stays alive as long as LLEvents
-          int tab_ref = lua_ref(info->L, -1);
-          LuaTable *listeners_tab = hvalue(luaA_toobject(info->L, -1));
-          lua_pop(info->L, 1);                                         /* ... */
-
+          // Create userdata with safe initial values FIRST to handle cycles
           auto *llevents = (lua_LLEvents*)lua_newuserdatataggedwithmetatable(
               info->L,
               sizeof(lua_LLEvents),
               UTAG_LLEVENTS
           );
                                                               /* ... llevents */
-          llevents->listeners_tab_ref = tab_ref;
-          llevents->listeners_tab = listeners_tab;
+          llevents->listeners_tab_ref = -1;
+          llevents->listeners_tab = nullptr;
+
+          // Register immediately to handle cycles (listeners_tab may reference this)
+          registerobject(info);
+
+          // Unpersist listeners_tab and store ref IMMEDIATELY to prevent leaks
+          unpersist(info);                           /* ... llevents handlers_tab */
+          eris_checktype(info, -1, LUA_TTABLE);
+          llevents->listeners_tab_ref = lua_ref(info->L, -1);
+          llevents->listeners_tab = hvalue(luaA_toobject(info->L, -1));
+          lua_pop(info->L, 1);                                    /* ... llevents */
 
-          // Manually put the LLEvents in the references table at the correct reference index
-          lua_pushvalue(info->L, -1);             /* perms reftbl ... obj obj */
-          lua_rawseti(info->L, REFTIDX, reference);   /* perms reftbl ... obj */
           break;
       }
       case UTAG_LLTIMERS:
       {
-          // Because we have an inner, wrapped table reference we need to reserve
-          // the idx for the outer timer manager first, since we saw it first.
-          int reference = allocate_ref_idx(info);
+          // Create userdata with safe initial values FIRST to handle cycles
+          auto *lltimers = (lua_LLTimers*)lua_newuserdatataggedwithmetatable(
+              info->L,
+              sizeof(lua_LLTimers),
+              UTAG_LLTIMERS
+          );
+                                                              /* ... lltimers */
+          lltimers->timers_tab_ref = -1;
+          lltimers->llevents_ref = -1;
+          lltimers->timer_wrapper_ref = -1;
+          lltimers->timers_tab = nullptr;
 
-          unpersist(info);                                  /* ... timers_tab */
+          // Register immediately to handle cycles (timer_wrapper has LLTimers as upvalue)
+          registerobject(info);
+
+          // Unpersist timers_tab and store ref IMMEDIATELY to prevent leaks
+          unpersist(info);                            /* ... lltimers timers_tab */
           eris_checktype(info, -1, LUA_TTABLE);
-          // We need to add a ref to the table so it stays alive as long as LLTimers
-          int tab_ref = lua_ref(info->L, -1);
-          LuaTable *timers_tab = hvalue(luaA_toobject(info->L, -1));
-          lua_pop(info->L, 1);                                         /* ... */
+          lltimers->timers_tab_ref = lua_ref(info->L, -1);
+          lltimers->timers_tab = hvalue(luaA_toobject(info->L, -1));
+          lua_pop(info->L, 1);                                     /* ... lltimers */
 
-          unpersist(info);                                   /* ... llevents */
+          // Unpersist llevents and store ref IMMEDIATELY
+          unpersist(info);                             /* ... lltimers llevents */
           eris_checkuserdatatag(info, -1, UTAG_LLEVENTS);
-          int llevents_ref = lua_ref(info->L, -1);
-          lua_pop(info->L, 1);                                         /* ... */
+          lltimers->llevents_ref = lua_ref(info->L, -1);
+          lua_pop(info->L, 1);                                     /* ... lltimers */
 
-          unpersist(info);                              /* ... timer_wrapper */
+          // Unpersist timer_wrapper and store ref IMMEDIATELY
+          unpersist(info);                        /* ... lltimers timer_wrapper */
           eris_checktype(info, -1, LUA_TFUNCTION);
-          int timer_wrapper_ref = lua_ref(info->L, -1);
-          lua_pop(info->L, 1);                                         /* ... */
+          lltimers->timer_wrapper_ref = lua_ref(info->L, -1);
+          lua_pop(info->L, 1);                                     /* ... lltimers */
 
-          auto *lltimers = (lua_LLTimers*)lua_newuserdatataggedwithmetatable(
-              info->L,
-              sizeof(lua_LLTimers),
-              UTAG_LLTIMERS
-          );
-                                                              /* ... lltimers */
-          lltimers->timers_tab_ref = tab_ref;
-          lltimers->timers_tab = timers_tab;
-          lltimers->llevents_ref = llevents_ref;
-          lltimers->timer_wrapper_ref = timer_wrapper_ref;
-
-          // Manually put the LLTimers in the references table at the correct reference index
-          lua_pushvalue(info->L, -1);             /* perms reftbl ... obj obj */
-          lua_rawseti(info->L, REFTIDX, reference);   /* perms reftbl ... obj */
           break;
       }
       default:

tests/conformance/lltimers.lua

@@ -345,4 +345,24 @@ incrementclock(0.3)
 LLEvents:_handleEvent('timer')
 assert(callable_count == 2)
 
+-- make sure serialization still works
+setclock(0)
+-- error() is the poor man's long-return.
+local function throw_error() error("called!") end
+LLTimers:on(0.5, throw_error)
+
+-- In reality you wouldn't give users primitives to clone these, but just for testing!
+local timers_clone = ares.unpersist(ares.persist(LLTimers))
+setclock(0.6)
+
+assert_errors(function() timers_clone:_tick() end, "called!")
+assert_errors(function() LLTimers:_tick() end, "called!")
+
+incrementclock(0.6)
+
+LLTimers:off(throw_error)
+-- Only one of them now has the problematic handler
+LLTimers:_tick()
+assert_errors(function() timers_clone:_tick() end, "called!")
+
 return "OK"