From 01f83215a2ad235fbf306f591c6c0547b1bb7047 Mon Sep 17 00:00:00 2001 From: Benoit Germain Date: Thu, 15 Nov 2018 11:20:14 +0100 Subject: Deep userdata must embed DeepPrelude to save an allocation (also changes Deep protocol) --- CHANGES | 3 +++ deep_test/deep_test.c | 3 ++- docs/index.html | 18 +++++++------- src/deep.c | 66 ++++++++++++++++++++------------------------------- src/deep.h | 12 ++++++---- src/keeper.c | 2 +- src/lanes.c | 6 +++-- src/lanes_private.h | 2 -- src/linda.c | 5 ++-- 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) 
	void* idfunc( lua_State* L, DeepOp op_);
op_ can be one of: Take a look at linda_id in lanes.c or deep_test_id in deep_test.c. @@ -1624,13 +1624,13 @@ int luaD_new_clonable( lua_State* L)

- Deep userdata in transit inside keeper states (sent in a linda but not yet consumed) don't call idfunc(eDO_delete) and aren't considered by reference counting. The rationale is the following: -
- 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. -
- 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 idfunc() is never called from a keeper state. -
- Therefore, Lanes can just call idfunc(eDO_delete) 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 idfunc(eDO_delete) and aren't considered by reference counting. The rationale is the following: +
+ 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. +
+ 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 idfunc() is never called from a keeper state. +
+ Therefore, Lanes can just call idfunc(eDO_delete) 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.

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; } -- cgit v1.2.3-55-g6feb