Persistent dataGet Function

[VisenDev]

While working on the new allocation scheme for structEntry, I've ran into the need for a persistent dataGet function (ie, one that does not clear its data)

Example code demonstrating use case

        .copy_value_and_alloc_new => {
        
            // PROBLEM: this memory handle shows up as "null" if the structEntry function is not called for a 
            // frame (eg, if its expander has been collapsed). 
            
            // This causes dvui to lose its handle on the allocated memory and need 
           // to realloc (leaking the old memory address in the process)
            var memory_handle = dvui.dataGet(null, box.widget().data().id, "memory_handle", []u8);
            if (memory_handle == null) {
                const len = @max(64, result.*.len * 2);
                const memory = try allocator.?.alloc(u8, len);
                @memset(memory, 0);
                std.mem.copyForwards(u8, memory, result.*);
                dvui.dataSet(null, box.widget().data().id, "memory_handle", memory);
                memory_handle = memory;
            }

            //WARNING: this could leak memory if result has been dynamically allocated
            result.* = memory_handle.?;
            const text_box = try dvui.textEntry(@src(), .{ .text = .{ .buffer = memory_handle.? } }, opt.dvui_opts);
            text_box.deinit();
        },

I need some way of storing data that won't drop values and thereby leak memory. (Or a way of testing whether the pointer located at result.* was allocated using the provided allocator parameter)

[david-vanderson]

I'm not understanding the nuance here. My mental model is that when a new textEntry is shown, we use dataGetSliceDefault() (with the default being what is already in result) and use the result directly as the buffer for textEntry (or equivalently, use textEntry in the way where it does that for you). Then, only if the contents have been changed, structEntry would handle copying that value into the result pointer (which might require reallocation).

Does that not work?

[VisenDev]

Does that not work?

That does not work. Here is some code some of my code for handling u8 slice allocations

    const ProvidedPointerTreatment = enum {
        mutate_value_and_realloc,
        mutate_value_in_place_only,
        display_only,
        copy_value_and_alloc_new,
    };

    comptime var treatment: ProvidedPointerTreatment = .display_only;
    comptime if (!alloc) {
        if (@typeInfo(T).Pointer.is_const) {
            //we can't allocate or mutate, so display
            treatment = .display_only;
        } else {
            //we can't allocate memory, so we mutate the existing buffer 
            treatment = .mutate_value_in_place_only;
        }
    } else {
        if (@typeInfo(T).Pointer.is_const) {
            //we can't mutate the existing buffer, so we copy its value and allocate a new buffer
            //we keep a handle on our new buffer using the dataGet api (so we know we have already allocated)
            treatment = .copy_value_and_alloc_new;
        } else {
            //We mutate in place and realloc if necessary
            treatment = .mutate_value_and_realloc;
        }
    };

Since we want allocations made in alloc mode to be persistent and made using the provided allocator, we can't use dataGet directly to allocate memory.

Rather we allocate using allocator.alloc() and then dataGet is used to store a copy of the pointer to the allocated memory. That way, we can access a mutable pointer to the memory.

See

if (memory_handle == null) { //memory handle is null, so we need to allocate
  const memory = try allocator.?.alloc(u8, len); //allocate new memory
  @memset(memory, 0);  //set it to 0
  std.mem.copyForwards(u8, memory, result.*);  //copy any values out of the preexisting buffer
  dvui.dataSet(null, box.widget().data().id, "memory_handle", memory); //store a handle to the mutable memory buffer
}

The problem is that if the user closes an expander or otherwise prevents the textFieldWidget from being called, the dataGet data is cleared. This means that when we next call this line

var memory_handle = dvui.dataGet(null, box.widget().data().id, "memory_handle", []u8);

memory_handle will be null - causing us to allocate again unnecessarily and thereby leak memory. Thus, to fix this, we need some way of using dataGet that won't lose its data.

Without allocating in the manner, it would be impossible to edit []const u8 text fields - even in alloc mode. Rendering things like your example:

            a_str: []const u8 = &[_]u8{0} ** 20,

impossible to edit with structEntry.

If we use dataGetSliceDefault() to allocate memory, we have several problems

• We don't actually use the provided allocator parameter to allocate memory

• Any allocated memory is immediately invalidated as soon as the textEntry is closed

  • For example, when editing a dvui.Theme any name: []const u8 font fields will immediately segfault upon the expander being closed - because the dataGetSlice memory is now invalid. If we want to be able to edit []const u8 font names we need to allocate memory using the allocator.

  This problem is why I disabled all the font fields in the dvui.Theme example. Currently we are using dataGetSlice to allocate memory for the font names. But this immediately causes a segfault once the dataGetSlice memory is freed

                 .font_body = .{ .disabled = true },
               .font_heading = .{ .disabled = true },
               .font_caption = .{ .disabled = true },
               .font_caption_heading = .{ .disabled = true },
               .font_title = .{ .disabled = true },
               .font_title_1 = .{ .disabled = true },
               .font_title_2 = .{ .disabled = true },
               .font_title_3 = .{ .disabled = true },
               .font_title_4 = .{ .disabled = true },

Hence we either need to allocate new persistent memory or become unable to generate fieldEntries for any [] const pointers (which is unfortunate since []const u8 is used for almost all text fields in zig)

I also spoke about this problem here

[david-vanderson]

Thanks for the detailed write up! It sounds like the issue is only with copy_value_and_alloc_new stuff right?

Can you commit what you have so far and leave that part commented out? I want to play with it. Either I'll figure out something or you'll get to see me bash my head against it. My goal is to be able to edit theme names (that start []const u8) and if they change, get backed by the passed allocator.

[VisenDev]

Thanks for the detailed write up! It sounds like the issue is only with copy_value_and_alloc_new stuff right?

Yes, the other situations don't have this problem, only copy_value_and_alloc_new (ie, const pointer in alloc mode). I tried to use an enum here so that it was really clear how memory would be treated in each situation

Can you commit what you have so far and leave that part commented out? I want to play with it. Either I'll figure out something or you'll get to see me bash my head against it.

Sounds good.

My goal is to be able to edit theme names (that start []const u8) and if they change, get backed by the passed allocator.

Yes, that is what I am trying to get to work as well. The case of font name input is what made me create #123

[david-vanderson]

Thanks very much for bearing with me. I think I am seeing the problem(s). In the example of editing a theme name (which starts out pointing to a read-only const string:

Problem 1: The user can't directly edit that string, so what does the user edit?
Answer: a temporary buffer allocated with dataGetDefault

We modify TextEntryWidget to detect user edits. If so, structEntryAlloc can alloc a new slice and point the theme struct name field to that.

Problem 2: How does structEntryAlloc know whether to free the old name field slice? This is the same as asking if this is the very first time the user has edited this field.

Let's imagine some crazy solution that fixes problem 2, like a global hashmap of slices that structEntryAlloc has alloced by Allocator.

Problem 3: When is it okay to free a slice we allocated to hold the new text?

In the theme name case, the answer is never, so effectively we would leak the last slice we allocated.

I think this is true for any const slice that didn't originally come from the allocator passed to structEntryAlloc. Even if we went to heroic lengths to solve problem 2, I can't see any way to solve problem 3 even in theory.

I think we have to say any const slice must be display only. Am I wrong?

[VisenDev]

I think we have to say any const slice must be display only. Am I wrong?

Yes, it might be best to make this restriction for simplicity's sake. I'm sure we could come up with some way to solve all these problems but that would probably be quite complex and make things more confusing.

[david-vanderson]

I agree. I was thinking of crazy things like having a button that says "this allows you to edit this field but leaks memory", but it just seems too much at least for now.