luarcu: add string support using external strings and kref by carloslack · Pull Request #512 · luainkernel/lunatik

carloslack · 2026-03-19T16:48:23Z

Add lunatik_string_t (kref + len + flexible char data[]) to lunatik_val.h
Extend lunatik_value_t union with lunatik_string_t *string member
lunatik_checkvalue: handle LUA_TSTRING by kmalloc'ing a lunatik_string_t, copying the string data out of Lua's GC reach, and kref_init'ing it
lunatik_pushvalue: handle LUA_TSTRING via lua_pushexternalstring with a custom alloc callback (lunatik_string_alloc) that kref_put's on GC; kref_get is called before push so Lua's GC holds one reference
Add lunatik_putvalue to release the caller's string reference after lunatik_pushvalue (no-op for objects, which transfer their ref)
luarcu_newentry/luarcu_free/luarcu_getvalue: mirror existing object kref_get/kref_put pattern for string values
luarcu_index: call lunatik_putvalue to release luarcu_getvalue's kref
luarcu_newindex: save ret before releasing string ref via lunatik_putvalue
luarcu_map: generalize to lunatik_value_t, handle string kref alongside existing object handling; refactor map_handle/map_call accordingly

- Add lunatik_string_t (kref + len + flexible char data[]) to lunatik_val.h - Extend lunatik_value_t union with lunatik_string_t *string member - lunatik_checkvalue: handle LUA_TSTRING by kmalloc'ing a lunatik_string_t, copying the string data out of Lua's GC reach, and kref_init'ing it - lunatik_pushvalue: handle LUA_TSTRING via lua_pushexternalstring with a custom alloc callback (lunatik_string_alloc) that kref_put's on GC; kref_get is called before push so Lua's GC holds one reference - Add lunatik_putvalue to release the caller's string reference after lunatik_pushvalue (no-op for objects, which transfer their ref) - luarcu_newentry/luarcu_free/luarcu_getvalue: mirror existing object kref_get/kref_put pattern for string values - luarcu_index: call lunatik_putvalue to release luarcu_getvalue's kref - luarcu_newindex: save ret before releasing string ref via lunatik_putvalue - luarcu_map: generalize to lunatik_value_t, handle string kref alongside existing object handling; refactor map_handle/map_call accordingly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

carloslack · 2026-03-19T18:18:54Z

+		const char *s = lua_tolstring(L, ix, &len);
+		lunatik_string_t *str = kmalloc(sizeof(*str) + len + 1, GFP_ATOMIC);
+		if (unlikely(!str))
+			luaL_argerror(L, ix, "not enough memory");


This one could be the upcoming lunatik_assert

we have enomem() already

carloslack · 2026-03-19T18:19:52Z

 		int boolean;
 		lua_Integer integer;
 		lunatik_object_t *object;
+		lunatik_string_t *string;


Here we have it added in the union. It can be checked with new lunatik_isstring
There the addition of lunatik_getstring, and lunatik_putstring that when called end up kfreeing the memory in the callback & update refcount.

carloslack · 2026-03-19T18:24:11Z

+		if (lunatik_isuserdata(&value))
+			lunatik_putobject(value.object);
+		else if (lunatik_isstring(&value))
+			lunatik_putstring(value.string);


Repeating code here - could be a macro (passing get or put) or an inline

lneto · 2026-03-20T13:11:16Z

 {
 	if (lunatik_isuserdata(&entry->value))
 		lunatik_putobject(entry->value.object);
+	else if (lunatik_isstring(&entry->value))


we can put value into a local var

lneto · 2026-03-20T13:12:00Z

 		*value = entry->value;
 		if (lunatik_isuserdata(value))
 			lunatik_getobject(value->object);
+		else if (lunatik_isstring(value))
+			lunatik_getstring(value->string);


this pattern is repeated quite a lot.. we can move it to a function.. perhaps on lunatik_val module?

lneto · 2026-03-20T13:12:22Z


 	luarcu_getvalue(table, key, keylen, &value);
 	lunatik_pushvalue(L, &value);
+	lunatik_putvalue(&value);


it doesn't seem right

lneto · 2026-03-20T13:13:05Z

 	lunatik_checkvalue(L, 3, &value);
-	if (luarcu_setvalue(table, key, keylen, &value) < 0)
+	int ret = luarcu_setvalue(table, key, keylen, &value);
+	lunatik_putvalue(&value);


also this.. why we are putting value here?

the string can leak because caller's ref is never released
after luarcu_setvalue I think the count is 2 (caller holds one)

lneto · 2026-03-20T13:14:16Z

-	lunatik_object_t *value = (lunatik_object_t *)lua_touserdata(L, 3);
+	lunatik_value_t *value = (lunatik_value_t *)lua_touserdata(L, 3);


was map() broken for values? we should have a separate fix for this, I think

also, it would be good to add a regression test..

@carloslack I added a fix for this here.. #515

lneto · 2026-03-20T13:18:46Z

+	case LUA_TSTRING: {
+		size_t len;
+		const char *s = lua_tolstring(L, ix, &len);
+		lunatik_string_t *str = kmalloc(sizeof(*str) + len + 1, GFP_ATOMIC);


we should use lunatik_malloc here, right? we would need to check if we have already a dup API, I got the impression we do have..

lneto · 2026-03-20T13:21:17Z

+
+static void *lunatik_string_alloc(void *ud, void *ptr, size_t osize, size_t nsize)
+{
+	if (nsize == 0)


I'm not sure it's enough.. we need to check the API contract..

lneto · 2026-03-20T13:21:36Z


+void lunatik_freestring(struct kref *kref)
+{
+	kfree(container_of(kref, lunatik_string_t, kref));


we also need to free the buffer, right?

lneto · 2026-03-20T13:22:35Z


+typedef struct lunatik_string_s {
+	struct kref kref;
+	size_t len;


do we really need size? we are dealing with null-terminated strings only

lneto · 2026-03-20T13:23:08Z

+#define lunatik_putstring(s)	kref_put(&(s)->kref, lunatik_freestring)
+
+/* release value's reference after lunatik_pushvalue (strings only; objects transfer their ref) */
+static inline void lunatik_putvalue(lunatik_value_t *value)


this one could abstract both string and object

- lunatik_val: add lunatik_holdvalue/lunatik_dropvalue helpers to abstract the repeated isuserdata/isstring get/put pattern - luarcu: use lunatik_holdvalue/lunatik_dropvalue throughout - lunatik_freestring: use lunatik_free instead of kfree for consistency - lunatik_checkvalue: extract lunatik_newstring helper (mirrors lunatik_checkobject), use lunatik_checkalloc instead of kmalloc Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Separate data buffer from struct: char *data instead of flexible array, allocated with lunatik_malloc so it can be freed explicitly - Drop len field: strings are null-terminated, strlen suffices - lunatik_freestring: explicitly free str->data then str - lunatik_pushvalue: copy data into a lunatik_malloc buffer and push via lunatik_pushstring; drop lunatik_string_alloc (no longer needed) - Use strncpy for all new string copies Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

carloslack · 2026-03-24T19:05:39Z

+void lunatik_freestring(struct kref *kref)
+{
+	lunatik_string_t *str = container_of(kref, lunatik_string_t, kref);
+	lunatik_free(str->data);


need to make sure data is valid..

carloslack · 2026-03-24T19:10:12Z

+	const char *s = luaL_checkstring(L, ix);
+	lunatik_string_t *str = lunatik_checkalloc(L, sizeof(*str));
+	str->data = lunatik_malloc(L, strlen(s) + 1);
+	if (unlikely(!str->data)) {


alright, it is handled here

carloslack · 2026-03-24T19:11:14Z

+#define lunatik_getstring(s)	kref_get(&(s)->kref)
+#define lunatik_putstring(s)	kref_put(&(s)->kref, lunatik_freestring)
+
+static inline void lunatik_holdvalue(lunatik_value_t *value)


could still be a local macro...

carloslack commented Mar 19, 2026

View reviewed changes

lneto reviewed Mar 20, 2026

View reviewed changes

Use lunatik_enomem instead of argcheck.

f9aea79

carloslack force-pushed the claude_str branch from b3a68d1 to f9aea79 Compare March 24, 2026 17:04

kovid-labs and others added 2 commits March 24, 2026 18:50

carloslack commented Mar 24, 2026

View reviewed changes

		lunatik_object_t value = (lunatik_object_t )lua_touserdata(L, 3);
		lunatik_value_t value = (lunatik_value_t )lua_touserdata(L, 3);

Conversation

carloslack commented Mar 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carloslack Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carloslack Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carloslack Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

carloslack Mar 19, 2026 •

edited

Loading

carloslack Mar 19, 2026 •

edited

Loading

carloslack Mar 24, 2026 •

edited

Loading