Deep userdata must embed DeepPrelude to save an allocation (also changes Deep protocol)

9 files changed, 56 insertions, 61 deletions

diff --git a/CHANGES b/CHANGES
index 1025484..713e516 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,8 @@
 CHANGES:
 
+CHANGE 137: BGe 15-Nov-18
+   * Deep userdata must embed DeepPrelude to save an allocation (also changes Deep protocol)
+
 CHANGE 136: BGe 15-Nov-18
    * split linda code in a separate file
    * rockspec for version v3.13.0
diff --git a/deep_test/deep_test.c b/deep_test/deep_test.c
index 4aac586..7edd33f 100644
--- a/deep_test/deep_test.c
+++ b/deep_test/deep_test.c
@@ -17,6 +17,7 @@
 
 struct s_MyDeepUserdata
 {
+        DeepPrelude prelude; // Deep userdata MUST start with this header
         lua_Integer val;
 };
 static void* deep_test_id( lua_State* L, enum eDeepOp op_);
@@ -67,6 +68,7 @@ static void* deep_test_id( lua_State* L, enum eDeepOp op_)
                 case eDO_new:
                 {
                         struct s_MyDeepUserdata* deep_test = (struct s_MyDeepUserdata*) malloc( sizeof(struct s_MyDeepUserdata));
+                        deep_test->prelude.magic.value = DEEP_VERSION.value;
                         deep_test->val = 0;
                         return deep_test;
                 }
@@ -81,7 +83,6 @@ static void* deep_test_id( lua_State* L, enum eDeepOp op_)
                 case eDO_metatable:
                 {
                         luaL_getmetatable( L, "deep");             // mt
-                        luaG_pushdeepversion( L);                  // mt version
                         return NULL;
                 }
 
diff --git a/docs/index.html b/docs/index.html
index ddb9ed3..723766d 100644
--- a/docs/index.html
+++ b/docs/index.html
@@ -1607,9 +1607,9 @@ int luaD_new_clonable( lua_State* L)
                 <table border="1" bgcolor="#E0E0FF" cellpadding="10" style="width:50%"><tr><td><pre>    void* idfunc( lua_State* L, DeepOp op_);</pre></td></tr></table>
                 <tt>op_</tt> can be one of:
                 <ul>
-                        <li><tt>eDO_new</tt>: requests the creation of a new object, whose pointer is returned.</li>
+                        <li><tt>eDO_new</tt>: requests the creation of a new object, whose pointer is returned. Starting with version 3.13.0, object should embed <tt>DeepPrelude</tt> structure as header and initialize its <tt>magic</tt> member with the current <tt>DEEP_VERSION</tt>.</li>
                         <li><tt>eDO_delete</tt>: receives this same pointer on the stack as a light userdata, and should cleanup the object.</li>
-                        <li><tt>eDO_metatable</tt>: should build a metatable for the object. Don't cache the metatable yourself, Lanes takes care of it (<tt>eDO_metatable</tt> should only be invoked once per state). Push the metatable on the stack, then call <tt>luaG_pushdeepversion()</tt> before returning (new in version 3.9.5).</li>
+                        <li><tt>eDO_metatable</tt>: should build a metatable for the object. Don't cache the metatable yourself, Lanes takes care of it (<tt>eDO_metatable</tt> should only be invoked once per state). Just push the metatable on the stack.</li>
                         <li><tt>eDO_module</tt>: requests the name of the module that exports the idfunc, to be returned. It is necessary so that Lanes can require it in any lane state that receives a userdata. This is to prevent crashes in situations where the module could be unloaded while the idfunc pointer is still held.</li>
                 </ul>
                 Take a look at <tt>linda_id</tt> in <tt>lanes.c</tt> or <tt>deep_test_id</tt> in <tt>deep_test.c</tt>.
@@ -1624,13 +1624,13 @@ int luaD_new_clonable( lua_State* L)
 </p>
 
 <p>
-        Deep userdata in transit inside keeper states (sent in a linda but not yet consumed) don't call <tt>idfunc(eDO_delete)</tt> and aren't considered by reference counting. The rationale is the following:
-        <br/>
-        If some non-keeper state holds a deep userdata for some deep object, then even if the keeper collects its own deep userdata, it shouldn't be cleaned up since the refcount is not 0.
-        <br/>
-        OTOH, if a keeper state holds the last deep userdata for some deep object, then no lane can do actual work with it. But as it happens, deep userdata are only copied to and from keeper states. Most notably, the object's <tt>idfunc()</tt> is never called from a keeper state.
-        <br/>
-        Therefore, Lanes can just call <tt>idfunc(eDO_delete)</tt> when the last non-keeper-held deep userdata is collected, as long as it doens't do the same in a keeper state after that, since any remaining deep userdata in keeper states now hold stale pointers.
+    Deep userdata in transit inside keeper states (sent in a linda but not yet consumed) don't call <tt>idfunc(eDO_delete)</tt> and aren't considered by reference counting. The rationale is the following:
+    <br />
+    If some non-keeper state holds a deep userdata for some deep object, then even if the keeper collects its own deep userdata, it shouldn't be cleaned up since the refcount is not 0.
+    <br />
+    OTOH, if a keeper state holds the last deep userdata for some deep object, then no lane can do actual work with it. Deep userdata's <tt>idfunc()</tt> is never called from a keeper state.
+    <br />
+    Therefore, Lanes can just call <tt>idfunc(eDO_delete)</tt> when the last non-keeper-held deep userdata is collected, as long as it doesn't do the same in a keeper state after that, since any remaining deep userdata in keeper states now hold stale pointers.
 </p>
 
 <p>
diff --git a/src/deep.c b/src/deep.c
index af7f580..c351bf7 100644
--- a/src/deep.c
+++ b/src/deep.c
@@ -105,16 +105,6 @@ void push_registry_subtable( lua_State* L, UniqueKey key_)
 
 /*---=== Deep userdata ===---*/
 
-void luaG_pushdeepversion( lua_State* L) { (void) lua_pushliteral( L, "ab8743e5-84f8-485d-9c39-008e84656188");}
-
-
-
-/* The deep portion must be allocated separately of any Lua state's; it's
-* lifespan may be longer than that of the creating state.
-*/
-#define DEEP_MALLOC malloc
-#define DEEP_FREE   free
-
 /* 
 * 'registry[REGKEY]' is a two-way lookup table for 'idfunc's and those type's
 * metatables:
@@ -211,10 +201,9 @@ static inline luaG_IdFunction get_idfunc( lua_State* L, int index, LookupMode mo
 void free_deep_prelude( lua_State* L, DeepPrelude* prelude_)
 {
         // Call 'idfunc( "delete", deep_ptr )' to make deep cleanup
-        lua_pushlightuserdata( L, prelude_->deep);
+        lua_pushlightuserdata( L, prelude_);
         ASSERT_L( prelude_->idfunc);
         prelude_->idfunc( L, eDO_delete);
-        DEEP_FREE( (void*) prelude_);
 }
 
 
@@ -276,7 +265,7 @@ char const* push_deep_proxy( Universe* U, lua_State* L, DeepPrelude* prelude, Lo
 
         // Check if a proxy already exists
         push_registry_subtable_mode( L, DEEP_PROXY_CACHE_KEY, "v");                                        // DPC
-        lua_pushlightuserdata( L, prelude->deep);                                                          // DPC deep
+        lua_pushlightuserdata( L, prelude);                                                                // DPC deep
         lua_rawget( L, -2);                                                                                // DPC proxy
         if ( !lua_isnil( L, -1))
         {
@@ -313,20 +302,13 @@ char const* push_deep_proxy( Universe* U, lua_State* L, DeepPrelude* prelude, Lo
                 // 1 - make one and register it
                 if( mode_ != eLM_ToKeeper)
                 {
-                        (void) prelude->idfunc( L, eDO_metatable);                                                     // DPC proxy metatable deepversion
-                        if( lua_gettop( L) - oldtop != 1 || !lua_istable( L, -2) || !lua_isstring( L, -1))
+                        (void) prelude->idfunc( L, eDO_metatable);                                                     // DPC proxy metatable
+                        if( lua_gettop( L) - oldtop != 0 || !lua_istable( L, -1))
                         {
                                 lua_settop( L, oldtop);                                                                      // DPC proxy X
                                 lua_pop( L, 3);                                                                              //
                                 return "Bad idfunc(eOP_metatable): unexpected pushed value";
                         }
-                        luaG_pushdeepversion( L);                                                                      // DPC proxy metatable deepversion deepversion
-                        if( !lua501_equal( L, -1, -2))
-                        {
-                                lua_pop( L, 5);                                                                              //
-                                return "Bad idfunc(eOP_metatable): mismatched deep version";
-                        }
-                        lua_pop( L, 2);                                                                                // DPC proxy metatable
                         // if the metatable contains a __gc, we will call it from our own
                         lua_getfield( L, -1, "__gc");                                                                  // DPC proxy metatable __gc
                 }
@@ -420,7 +402,7 @@ char const* push_deep_proxy( Universe* U, lua_State* L, DeepPrelude* prelude, Lo
         lua_setmetatable( L, -2);                                                                          // DPC proxy
 
         // If we're here, we obviously had to create a new proxy, so cache it.
-        lua_pushlightuserdata( L, (*proxy)->deep);                                                         // DPC proxy deep
+        lua_pushlightuserdata( L, prelude);                                                                // DPC proxy deep
         lua_pushvalue( L, -2);                                                                             // DPC proxy deep proxy
         lua_rawset( L, -4);                                                                                // DPC proxy
         lua_remove( L, -2);                                                                                // proxy
@@ -454,34 +436,38 @@ char const* push_deep_proxy( Universe* U, lua_State* L, DeepPrelude* prelude, Lo
 int luaG_newdeepuserdata( lua_State* L, luaG_IdFunction idfunc)
 {
         char const* errmsg;
-        DeepPrelude* prelude = DEEP_MALLOC( sizeof( DeepPrelude));
-        if( prelude == NULL)
-        {
-                return luaL_error( L, "couldn't not allocate deep prelude: out of memory");
-        }
-
-        prelude->refcount = 0; // 'push_deep_proxy' will lift it to 1
-        prelude->idfunc = idfunc;
 
         STACK_GROW( L, 1);
         STACK_CHECK( L);
         {
                 int oldtop = lua_gettop( L);
-                prelude->deep = idfunc( L, eDO_new);
-                if( prelude->deep == NULL)
+                DeepPrelude* prelude = idfunc( L, eDO_new);
+                if( prelude == NULL)
                 {
                         luaL_error( L, "idfunc(eDO_new) failed to create deep userdata (out of memory)");
                 }
+                if( prelude->magic.value != DEEP_VERSION.value)
+                {
+                        // just in case, don't leak the newly allocated deep userdata object
+                        lua_pushlightuserdata( L, prelude);
+                        idfunc( L, eDO_delete);
+                        return luaL_error( L, "Bad idfunc(eDO_new): DEEP_VERSION is incorrect, rebuild your implementation with the latest deep implementation");
+                }
+                prelude->refcount = 0; // 'push_deep_proxy' will lift it to 1
+                prelude->idfunc = idfunc;
 
                 if( lua_gettop( L) - oldtop != 0)
                 {
-                        luaL_error( L, "Bad idfunc(eDO_new): should not push anything on the stack");
+                        // just in case, don't leak the newly allocated deep userdata object
+                        lua_pushlightuserdata( L, prelude);
+                        idfunc( L, eDO_delete);
+                        return luaL_error( L, "Bad idfunc(eDO_new): should not push anything on the stack");
+                }
+                errmsg = push_deep_proxy( universe_get( L), L, prelude, eLM_LaneBody);  // proxy
+                if( errmsg != NULL)
+                {
+                        return luaL_error( L, errmsg);
                 }
-        }
-        errmsg = push_deep_proxy( universe_get( L), L, prelude, eLM_LaneBody);  // proxy
-        if( errmsg != NULL)
-        {
-                luaL_error( L, errmsg);
         }
         STACK_END( L, 1);
         return 1;
@@ -508,7 +494,7 @@ void* luaG_todeep( lua_State* L, luaG_IdFunction idfunc, int index)
         proxy = (DeepPrelude**) lua_touserdata( L, index);
         STACK_END( L, 0);
 
-        return (*proxy)->deep;
+        return *proxy;
 }
 
 
diff --git a/src/deep.h b/src/deep.h
index 918de6a..8d06395 100644
--- a/src/deep.h
+++ b/src/deep.h
@@ -8,6 +8,7 @@
 
 #include "lua.h"
 #include "platform.h"
+#include "uniquekey.h"
 
 // forwards
 struct s_Universe;
@@ -42,13 +43,17 @@ typedef void* (*luaG_IdFunction)( lua_State* L, DeepOp op_);
 
 // ################################################################################################
 
-// this is pointed to by full userdata proxies, and allocated with malloc() to survive any lua_State lifetime
+// crc64/we of string "DEEP_VERSION_1" generated at http://www.nitrxgen.net/hashgen/
+static DECLARE_CONST_UNIQUE_KEY( DEEP_VERSION, 0x4f4eadf0accf6c73);
+
+// should be used as header for full userdata
 struct s_DeepPrelude
 {
-        volatile int refcount;
-        void* deep;
+        DECLARE_UNIQUE_KEY( magic); // must be filled by the Deep userdata idfunc that allocates it on eDO_new operation
         // when stored in a keeper state, the full userdata doesn't have a metatable, so we need direct access to the idfunc
         luaG_IdFunction idfunc;
+        // data is destroyed when refcount is 0
+        volatile int refcount;
 };
 typedef struct s_DeepPrelude DeepPrelude;
 
@@ -57,6 +62,5 @@ void free_deep_prelude( lua_State* L, DeepPrelude* prelude_);
 
 extern LANES_API int luaG_newdeepuserdata( lua_State* L, luaG_IdFunction idfunc);
 extern LANES_API void* luaG_todeep( lua_State* L, luaG_IdFunction idfunc, int index);
-extern LANES_API void luaG_pushdeepversion( lua_State* L);
 
 #endif // __LANES_DEEP_H__
diff --git a/src/keeper.c b/src/keeper.c
index 091463e..0471cb7 100644
--- a/src/keeper.c
+++ b/src/keeper.c
@@ -754,7 +754,7 @@ void keeper_release( Keeper* K)
         if( K) MUTEX_UNLOCK( &K->keeper_cs);
 }
 
-void keeper_toggle_nil_sentinels( lua_State* L, int val_i_, LookupMode mode_)
+void keeper_toggle_nil_sentinels( lua_State* L, int val_i_, LookupMode const mode_)
 {
         int i, n = lua_gettop( L);
         for( i = val_i_; i <= n; ++ i)
diff --git a/src/lanes.c b/src/lanes.c
index f2e3065..037e44f 100644
--- a/src/lanes.c
+++ b/src/lanes.c
@@ -733,7 +733,10 @@ static int selfdestruct_gc( lua_State* L)
         // necessary so that calling free_deep_prelude doesn't crash because linda_id expects a linda lightuserdata at absolute slot 1
         lua_settop( L, 0);
         // no need to mutex-protect this as all threads in the universe are gone at that point
-        -- U->timer_deep->refcount; // should be 0 now
+        if( U->timer_deep != NULL) // test ins case some early internal error prevented Lanes from creating the deep timer
+        {
+                -- U->timer_deep->refcount; // should be 0 now
+        }
         free_deep_prelude( L, (DeepPrelude*) U->timer_deep);
         U->timer_deep = NULL;
 
@@ -2162,7 +2165,6 @@ LUAG_FUNC( configure)
 
                 // Proxy userdata contents is only a 'DEEP_PRELUDE*' pointer
                 U->timer_deep = *(DeepPrelude**) lua_touserdata( L, -1);
-                ASSERT_L( U->timer_deep && (U->timer_deep->refcount == 1) && U->timer_deep->deep && U->timer_deep->idfunc == linda_id);
                 // increment refcount that this linda remains alive as long as the universe is.
                 ++ U->timer_deep->refcount;
                 lua_pop( L, 1);                                                                    // settings
diff --git a/src/lanes_private.h b/src/lanes_private.h
index a7e21d7..1adfa31 100644
--- a/src/lanes_private.h
+++ b/src/lanes_private.h
@@ -112,6 +112,4 @@ static inline int cancel_error( lua_State* L)
         return lua_error( L); // doesn't return
 }
 
-
-
 #endif // __lanes_private_h__
\ No newline at end of file
diff --git a/src/linda.c b/src/linda.c
index 98d2a8e..ee60ebc 100644
--- a/src/linda.c
+++ b/src/linda.c
@@ -46,6 +46,7 @@ THE SOFTWARE.
 */
 struct s_Linda
 {
+        DeepPrelude prelude; // Deep userdata MUST start with this header
         SIGNAL_T read_happened;
         SIGNAL_T write_happened;
         Universe* U; // the universe this linda belongs to
@@ -794,6 +795,7 @@ static void* linda_id( lua_State* L, DeepOp op_)
                         s = (struct s_Linda*) malloc( sizeof(struct s_Linda) + name_len); // terminating 0 is already included
                         if( s)
                         {
+                                s->prelude.magic.value = DEEP_VERSION.value;
                                 SIGNAL_INIT( &s->read_happened);
                                 SIGNAL_INIT( &s->write_happened);
                                 s->U = universe_get( L);
@@ -895,8 +897,7 @@ static void* linda_id( lua_State* L, DeepOp op_)
                         push_unique_key( L, NIL_SENTINEL);
                         lua_setfield(L, -2, "null");
 
-                        luaG_pushdeepversion( L);
-                        STACK_END( L, 2);
+                        STACK_END( L, 1);
                         return NULL;
                 }