CPlayerManager lineBuffer accessed while being updated with multiple byte Unicode characters

[assertivist]

uncaught exception of type utf8::not_enough_room: Not enough space

To reproduce: start a game, enter chat mode, type "option-L" or similar

Immediate crash, of everyone connected

This could have to do with the inputBuffer field of CPlayerManager:

Avara/src/game/CPlayerManager.cpp

Lines 258 to 263 in a8352bb

	case SDL_TEXTINPUT:
	if (keyboardActive) {
	for(char* a_char = event.text.text; *a_char; ++a_char) {
	inputBuffer.push_back(*a_char);
	}
	}

In any case, whether or not we fix the underlying issue, the exception could be caught and discarded

[rherriman]

Another similar crash occurs out-of-game, but only after you press Enter. (I can option-L multiple times without crashing before then.) Suspect this particular crash has to do with the chat tab. Here's the backtrace:

* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x00007fff702ab33a libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff70367e60 libsystem_pthread.dylib`pthread_kill + 430
    frame #2: 0x00007fff70232808 libsystem_c.dylib`abort + 120
    frame #3: 0x00007fff6d492458 libc++abi.dylib`abort_message + 231
    frame #4: 0x00007fff6d4838a7 libc++abi.dylib`demangling_terminate_handler() + 238
    frame #5: 0x00007fff6efbe5b1 libobjc.A.dylib`_objc_terminate() + 104
    frame #6: 0x00007fff6d491887 libc++abi.dylib`std::__terminate(void (*)()) + 8
    frame #7: 0x00007fff6d4941a2 libc++abi.dylib`__cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 27
    frame #8: 0x00007fff6d494169 libc++abi.dylib`__cxa_throw + 113
    frame #9: 0x00007fff6d4750c8 libc++.1.dylib`std::__1::__throw_out_of_range(char const*) + 56
    frame #10: 0x00007fff6d466b0e libc++.1.dylib`std::__1::__basic_string_common<true>::__throw_out_of_range() const + 16
    frame #11: 0x00007fff6d466daf libc++.1.dylib`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long, unsigned long, std::__1::allocator<char> const&) + 195
    frame #12: 0x00000001000f34f7 Avara`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::substr(this="¬", __pos=3, __n=18446744073709551615) const at string:3280:12
    frame #13: 0x00000001000f33b5 Avara`CPlayerManagerImpl::GetChatLine(this=0x0000000102905000) at CPlayerManager.cpp:745:20
    frame #14: 0x00000001000f2d47 Avara`CPlayerManagerImpl::RosterMessageText(this=0x0000000102905000, len=0, c="    \"normal\": [\n        -0.11374074921797517,\n        -0.9801174944686045,\n        0.1624628061341268\n      ],\n      \"tris\": [\n?-\x89\x01\x01") at CPlayerManager.cpp:710:90
    frame #15: 0x00000001000d862c Avara`CNetManager::ReceiveRosterMessage(this=0x00000001022a5ee0, slotId=0, len=1, c="\r    \"normal\": [\n        -0.11374074921797517,\n        -0.9801174944686045,\n        0.1624628061341268\n      ],\n      \"tris\": [\n?-\x89\x01\x01") at CNetManager.cpp:349:20
    frame #16: 0x0000000100026c34 Avara`CProtoControl::DelayedPacketHandler(this=0x000000010228d1c0, thePacket=0x0000000101892c80) at CProtoControl.cpp:89:21
    frame #17: 0x0000000100026879 Avara`DelayedProtoHandler(thePacket=0x0000000101892c80, userData="\x98K.") at CProtoControl.cpp:37:24
    frame #18: 0x000000010000ee24 Avara`CCommManager::ProcessQueue(this=0x0000000100f9d990) at CCommManager.cpp:363:33
    frame #19: 0x00000001000d7a41 Avara`CNetManager::ProcessQueue(this=0x00000001022a5ee0) at CNetManager.cpp:188:21
    frame #20: 0x00000001000c3360 Avara`CAvaraGame::GameTick(this=0x0000000102911c00) at CAvaraGame.cpp:815:13
    frame #21: 0x000000010005c3d9 Avara`CAvaraAppImpl::idle(this=0x00000001022612a0) at CAvaraApp.cpp:105:17
    frame #22: 0x0000000100220678 Avara`nanogui::mainloop(refresh=16) at common.cpp:82:25
    frame #23: 0x00000001002c02f7 Avara`main(argc=1, argv=0x00007ffeefbff5d8) at Avara.cpp:67:5
    frame #24: 0x00007fff70163cc9 libdyld.dylib`start + 1
    frame #25: 0x00007fff70163cc9 libdyld.dylib`start + 1

[rherriman]

Addendum to what I said above... typing ":¬)" followed by Enter does not crash.

[rherriman]

The issue I clumsily tacked onto this thread has been moved to #73.

[assertivist]

The code in the issue is likely not the source of the exception. But there are two other places here, where utf8 namespace code actually gets called, so it's likely from this area:

Avara/src/game/CPlayerManager.cpp

Lines 702 to 706 in be507a4

	case 8:
	if (lineBuffer.size()) {
	auto i = lineBuffer.end();
	utf8::previous(i, lineBuffer.begin());
	lineBuffer = std::deque<char>(lineBuffer.begin(), i);

Avara/src/game/CPlayerManager.cpp

Lines 721 to 729 in be507a4

	default:
	if (theChar >= 32) {
	lineBuffer.push_back(theChar);
	if (lineBuffer.size() > 220) {
	// FlushMessageText(true);
	auto i = lineBuffer.begin();
	utf8::advance(i, 55, lineBuffer.end());
	lineBuffer = std::deque<char>(i, lineBuffer.end());
	}

[assertivist]

lineBuffer and inputBuffer should probably be std::string instead of std::deque<char>. I used a deque because it was 'easier' to push and then pop characters FILO for sending over the network.

[assertivist]

I found out that the exception is thrown when attempting to read the length of lineBuffer. The calls are logically sound, what's happening is, my shortcuts to generate the special characters on other platforms are running and adding multiple unicode bytes at once to lineBuffer--however, the game thread is running CHUD, which is calling GetChatLine, which is trying to read the length of lineBuffer at the same time. When the distance function gets called with partial codepoints at the end, it throws that exception.

For now I have caught the exception so at least the app doesn't crash. But we probably need to lock lineBuffer when we're messing with it. I don't think adding a couple chars to a deque<char> should take very long, so I'm thinking it should be OK to block CHUD until we're done adding bytes.