Crash with double free when removing lines after appending bbcode

[rxn7]

Godot version

v3.4.4.stable.mono.official [419e713]

System information

linux 5.17.4-arch1-1

Issue description

This code crashes Godot Mono:

private void BbcodeTest() {
	while(GetLineCount() > 1)
		RemoveLine(0);
		
	for(int i=0; i<20; ++i) {
		AppendBbcode("[color=red]test[/color]\n");
	}
}

And this doesn't:

private void TextTest() {
	while(GetLineCount() > 1)
		RemoveLine(0);
	
	for(int i=0; i<20; ++i) {
		Text += "test\n";
	}
}

So it happens only when appending Bbcode.

Steps to reproduce

• Remove some lines from RichTextLabel
• Append Bbcode

Minimal reproduction project

bbcode_bug.zip

[rxn7]

=================================================================
	Managed Stacktrace:
=================================================================
	  at <unknown> <0xffffffff>
	  at Godot.NativeCalls:godot_icall_1_35 <0x000c1>
	  at Godot.RichTextLabel:RemoveLine <0x000bf>
	  at Test:BbcodeTest <0x00097>
	  at Test:_Process <0x000cb>
	  at <Module>:runtime_invoke_void__this___single <0x000cc>
=================================================================

This it the stacktrace I get after a crash

[akien-mga]

I can confirm it, it's not Mono related though, it's reproducible with GDScript:

extends RichTextLabel

func _process(delta):
	while get_line_count() > 1:
		remove_line(0)

	for i in range(20):
		append_bbcode("[color=red]test[/color]\n")

This doesn't happen with the same logic in _ready, but doing it in _process means doing it once per frame which is a lot for heavy bbcode operations, so it might have some race condition.

GDScript MRP: bbcode_bug.zip

The crash is reproducible in 3.x (73e6b7c) with this stacktrace on Linux:

double free or corruption (fasttop)

Thread 1 "godot-3.x" received signal SIGABRT, Aborted.
0x00007ffff7b3404c in __pthread_kill_implementation () from /lib64/libc.so.6

(gdb) bt
#0  0x00007ffff7b3404c in __pthread_kill_implementation () from /lib64/libc.so.6
#1  0x00007ffff7ae72f2 in raise () from /lib64/libc.so.6
#2  0x00007ffff7ad345c in abort () from /lib64/libc.so.6
#3  0x00007ffff7b28a48 in __libc_message () from /lib64/libc.so.6
#4  0x00007ffff7b3ceaa in malloc_printerr () from /lib64/libc.so.6
#5  0x00007ffff7b3e9ba in _int_free () from /lib64/libc.so.6
#6  0x00007ffff7b40f8b in free () from /lib64/libc.so.6
#7  0x00000000048bd428 in Memory::free_static (p_ptr=0x8479f50, p_pad_align=false) at core/os/memory.cpp:168
#8  0x000000000385d776 in memdelete<RichTextLabel::Item> (p_class=0x8479f50) at ./core/os/memory.h:117
#9  0x000000000384d5d4 in RichTextLabel::_remove_item (this=0x847aa10, p_item=0x8479f50, p_line=0, p_subitem_line=0) at scene/gui/rich_text_label.cpp:1715
#10 0x000000000384db0a in RichTextLabel::remove_line (this=0x847aa10, p_line=0) at scene/gui/rich_text_label.cpp:1780
#11 0x0000000002808573 in MethodBind1R<bool, int>::call (this=0x7c9ed30, p_object=0x847aa10, p_args=0x7fffffffb128, p_arg_count=1, r_error=...) at ./core/method_bind.gen.inc:941
#12 0x000000000476e46a in Object::call (this=0x847aa10, p_method=..., p_args=0x7fffffffb128, p_argcount=1, r_error=...) at core/object.cpp:918
#13 0x000000000480cb03 in Variant::call_ptr (this=0x7fffffffb350, p_method=..., p_args=0x7fffffffb128, p_argcount=1, r_ret=0x0, r_error=...) at core/variant_call.cpp:1193
#14 0x0000000001c9b39e in GDScriptFunction::call (this=0x84e25e0, p_instance=0x8203130, p_args=0x7fffffffc498, p_argcount=1, r_err=..., p_state=0x0) at modules/gdscript/gdscript_function.cpp:1050
#15 0x0000000001c49580 in GDScriptInstance::call_multilevel (this=0x8203130, p_method=..., p_args=0x7fffffffc498, p_argcount=1) at modules/gdscript/gdscript.cpp:1211
#16 0x00000000036699da in Node::_notification (this=0x847aa10, p_notification=16) at scene/main/node.cpp:63
#17 0x00000000016d450e in Node::_notificationv (this=0x847aa10, p_notification=16, p_reversed=false) at ./scene/main/node.h:45
#18 0x00000000016d4b39 in CanvasItem::_notificationv (this=0x847aa10, p_notification=16, p_reversed=false) at ./scene/2d/canvas_item.h:163
#19 0x00000000016d51d5 in Control::_notificationv (this=0x847aa10, p_notification=16, p_reversed=false) at ./scene/gui/control.h:47
#20 0x000000000385c889 in RichTextLabel::_notificationv (this=0x847aa10, p_notification=16, p_reversed=false) at scene/gui/rich_text_label.h:38
#21 0x000000000476e587 in Object::notification (this=0x847aa10, p_notification=16, p_reversed=false) at core/object.cpp:927
#22 0x00000000036a1bc3 in SceneTree::_notify_group_pause (this=0x82ec170, p_group=..., p_notification=16) at scene/main/scene_tree.cpp:1057
#23 0x000000000369eef0 in SceneTree::iteration (this=0x82ec170, p_time=0.0166666675) at scene/main/scene_tree.cpp:552
#24 0x00000000016adee1 in Main::iteration () at main/main.cpp:2244
#25 0x0000000001670acb in OS_X11::run (this=0x7fffffffcd50) at platform/x11/os_x11.cpp:3933
#26 0x000000000165ccaf in main (argc=1, argv=0x7fffffffd688) at platform/x11/godot_x11.cpp:55

It doesn't crash in master, so the bug might be fixed there already.

CC @bruvzg

[rxn7]

This doesn't happen with the same logic in _ready, but doing it in _process means doing it once per frame which is a lot for heavy bbcode operations, so it might have some race condition.

It doesn't happen in _ready because at that point there's no bbcode in RichTextLabel yet,I think that the problem is more related to removing bbcode lines than appending bbcode

[akien-mga]

Indeed, here's a more minimal reproducer:

extends RichTextLabel

func _ready():
	append_bbcode("[color=red]test[/color]\n")
	remove_line(0)

[timothyqiu]

Similar crash: #59935

[bruvzg]

It's fixed in master by #49060, seem like a fix can be used for 3.x as well (with a minor changes).

[akien-mga]

Fixed by #60618.