Fix client not quitting after graphics assertion

After the error message popup is shown for graphics assertions, the client window was destroyed but the process was not terminated properly. Now the graphics assertion error is handled like a normal assertion error, as those are also shown in an error message popup and correctly cause the client to break into the debugger or terminate.

However, calling `dbg_assert` while already owning the lock that `WaitForIdle` waits on will cause a deadlock, so error handling must be delayed until after the lock is released.

The buffer size for assertion messages is increased, as it was not sufficient for some graphics assertions.

The `ICommandProcessor::GetError` and `ICommandProcessor::GetWarning` functions are marked as `const`.

src/base/system.cpp

@@ -191,7 +191,7 @@ void dbg_assert_imp(const char *filename, int line, int test, const char *msg)
 	{
 		const bool already_failing = dbg_assert_has_failed();
 		dbg_assert_failing.store(true, std::memory_order_release);
-		char error[256];
+		char error[512];
 		str_format(error, sizeof(error), "%s(%d): %s", filename, line, msg);
 		dbg_msg("assert", "%s", error);
 		if(!already_failing)

src/engine/client/backend_sdl.cpp

@@ -108,30 +108,33 @@ void CGraphicsBackend_Threaded::StopProcessor()
 
 void CGraphicsBackend_Threaded::RunBuffer(CCommandBuffer *pBuffer)
 {
+	SGFXErrorContainer Error;
 #ifdef CONF_WEBASM
 	// run everything single threaded for now, context binding in a thread seems to not work as of now
-	if(!m_pProcessor->HasError())
+	Error = m_pProcessor->GetError();
+	if(Error.m_ErrorType == GFX_ERROR_TYPE_NONE)
 	{
 		RunBufferSingleThreadedUnsafe(pBuffer);
 	}
-	else
-	{
-		ProcessError();
-	}
 #else
 	WaitForIdle();
+	{
 		std::unique_lock<std::mutex> Lock(m_BufferSwapMutex);
-	if(!m_pProcessor->HasError())
+		Error = m_pProcessor->GetError();
+		if(Error.m_ErrorType == GFX_ERROR_TYPE_NONE)
 		{
 			m_pBuffer = pBuffer;
 			m_BufferInProcess.store(true, std::memory_order_relaxed);
 			m_BufferSwapCond.notify_all();
 		}
-	else
-	{
-		ProcessError();
 	}
 #endif
+
+	// Process error after lock is released to prevent deadlock
+	if(Error.m_ErrorType != GFX_ERROR_TYPE_NONE)
+	{
+		ProcessError(Error);
+	}
 }
 
 void CGraphicsBackend_Threaded::RunBufferSingleThreadedUnsafe(CCommandBuffer *pBuffer)
@@ -150,25 +153,23 @@ void CGraphicsBackend_Threaded::WaitForIdle()
 	m_BufferSwapCond.wait(Lock, [this]() { return m_pBuffer == nullptr; });
 }
 
-void CGraphicsBackend_Threaded::ProcessError()
+void CGraphicsBackend_Threaded::ProcessError(const SGFXErrorContainer &Error)
 {
-	const auto &Error = m_pProcessor->GetError();
+	std::string VerboseStr = "Graphics Assertion:";
-	std::string VerboseStr;
 	for(const auto &ErrStr : Error.m_vErrors)
 	{
+		VerboseStr.append("\n");
 		if(ErrStr.m_RequiresTranslation)
-			VerboseStr.append(std::string(m_TranslateFunc(ErrStr.m_Err.c_str(), "")) + "\n");
+			VerboseStr.append(m_TranslateFunc(ErrStr.m_Err.c_str(), ""));
 		else
-			VerboseStr.append(ErrStr.m_Err + "\n");
+			VerboseStr.append(ErrStr.m_Err);
 	}
-	const bool CreatedMsgBox = ShowMessageBox(SDL_MESSAGEBOX_ERROR, "Graphics Assertion", VerboseStr.c_str());
+	dbg_assert(false, VerboseStr.c_str());
-	// check if error msg can be shown, then assert
-	dbg_assert(!CreatedMsgBox, VerboseStr.c_str());
 }
 
 bool CGraphicsBackend_Threaded::GetWarning(std::vector<std::string> &WarningStrings)
 {
-	if(HasWarning())
+	if(m_Warning.m_WarningType != GFX_WARNING_TYPE_NONE)
 	{
 		m_Warning.m_WarningType = GFX_WARNING_TYPE_NONE;
 		WarningStrings = m_Warning.m_vWarnings;
@@ -270,49 +271,47 @@ bool CCommandProcessorFragment_SDL::RunCommand(const CCommandBuffer::SCommand *p
 
 void CCommandProcessor_SDL_GL::HandleError()
 {
-	auto &Error = GetError();
+	switch(m_Error.m_ErrorType)
-	switch(Error.m_ErrorType)
 	{
 	case GFX_ERROR_TYPE_INIT:
-		Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Failed during initialization. Try to change gfx_backend to OpenGL or Vulkan in settings_ddnet.cfg in the config directory and try again.", "Graphics error")});
+		m_Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Failed during initialization. Try to change gfx_backend to OpenGL or Vulkan in settings_ddnet.cfg in the config directory and try again.", "Graphics error")});
 		break;
 	case GFX_ERROR_TYPE_OUT_OF_MEMORY_IMAGE:
 		[[fallthrough]];
 	case GFX_ERROR_TYPE_OUT_OF_MEMORY_BUFFER:
 		[[fallthrough]];
 	case GFX_ERROR_TYPE_OUT_OF_MEMORY_STAGING:
-		Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Out of VRAM. Try removing custom assets (skins, entities, etc.), especially those with high resolution.", "Graphics error")});
+		m_Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Out of VRAM. Try removing custom assets (skins, entities, etc.), especially those with high resolution.", "Graphics error")});
 		break;
 	case GFX_ERROR_TYPE_RENDER_RECORDING:
-		Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("An error during command recording occurred. Try to update your GPU drivers.", "Graphics error")});
+		m_Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("An error during command recording occurred. Try to update your GPU drivers.", "Graphics error")});
 		break;
 	case GFX_ERROR_TYPE_RENDER_CMD_FAILED:
-		Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("A render command failed. Try to update your GPU drivers.", "Graphics error")});
+		m_Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("A render command failed. Try to update your GPU drivers.", "Graphics error")});
 		break;
 	case GFX_ERROR_TYPE_RENDER_SUBMIT_FAILED:
-		Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Submitting the render commands failed. Try to update your GPU drivers.", "Graphics error")});
+		m_Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Submitting the render commands failed. Try to update your GPU drivers.", "Graphics error")});
 		break;
 	case GFX_ERROR_TYPE_SWAP_FAILED:
-		Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Failed to swap framebuffers. Try to update your GPU drivers.", "Graphics error")});
+		m_Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Failed to swap framebuffers. Try to update your GPU drivers.", "Graphics error")});
 		break;
 	case GFX_ERROR_TYPE_UNKNOWN:
 		[[fallthrough]];
 	default:
-		Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Unknown error. Try to change gfx_backend to OpenGL or Vulkan in settings_ddnet.cfg in the config directory and try again.", "Graphics error")});
+		m_Error.m_vErrors.emplace_back(SGFXErrorContainer::SError{true, Localizable("Unknown error. Try to change gfx_backend to OpenGL or Vulkan in settings_ddnet.cfg in the config directory and try again.", "Graphics error")});
 		break;
 	}
 }
 
 void CCommandProcessor_SDL_GL::HandleWarning()
 {
-	auto &Warn = GetWarning();
+	switch(m_Warning.m_WarningType)
-	switch(Warn.m_WarningType)
 	{
 	case GFX_WARNING_TYPE_INIT_FAILED:
-		Warn.m_vWarnings.emplace_back(Localizable("Could not initialize the given graphics backend, reverting to the default backend now.", "Graphics error"));
+		m_Warning.m_vWarnings.emplace_back(Localizable("Could not initialize the given graphics backend, reverting to the default backend now.", "Graphics error"));
 		break;
 	case GFX_WARNING_TYPE_INIT_FAILED_MISSING_INTEGRATED_GPU_DRIVER:
-		Warn.m_vWarnings.emplace_back(Localizable("Could not initialize the given graphics backend, this is probably because you didn't install the driver of the integrated graphics card.", "Graphics error"));
+		m_Warning.m_vWarnings.emplace_back(Localizable("Could not initialize the given graphics backend, this is probably because you didn't install the driver of the integrated graphics card.", "Graphics error"));
 		break;
 	case GFX_WARNING_MISSING_EXTENSION:
 		// ignore this warning for now
@@ -321,7 +320,7 @@ void CCommandProcessor_SDL_GL::HandleWarning()
 		// ignore this warning for now
 		return;
 	default:
-		dbg_msg("gfx", "unhandled warning %d", (int)Warn.m_WarningType);
+		dbg_msg("gfx", "unhandled warning %d", (int)m_Warning.m_WarningType);
 		break;
 	}
 }
@@ -417,7 +416,7 @@ CCommandProcessor_SDL_GL::~CCommandProcessor_SDL_GL()
 	delete m_pGLBackend;
 }
 
-SGFXErrorContainer &CCommandProcessor_SDL_GL::GetError()
+const SGFXErrorContainer &CCommandProcessor_SDL_GL::GetError() const
 {
 	return m_Error;
 }
@@ -427,7 +426,7 @@ void CCommandProcessor_SDL_GL::ErroneousCleanup()
 	return m_pGLBackend->ErroneousCleanup();
 }
 
-SGFXWarningContainer &CCommandProcessor_SDL_GL::GetWarning()
+const SGFXWarningContainer &CCommandProcessor_SDL_GL::GetWarning() const
 {
 	return m_Warning;
 }

src/engine/client/backend_sdl.h

@@ -58,20 +58,10 @@ public:
 		virtual ~ICommandProcessor() = default;
 		virtual void RunBuffer(CCommandBuffer *pBuffer) = 0;
 
-		virtual SGFXErrorContainer &GetError() = 0;
+		virtual const SGFXErrorContainer &GetError() const = 0;
 		virtual void ErroneousCleanup() = 0;
 
-		virtual SGFXWarningContainer &GetWarning() = 0;
+		virtual const SGFXWarningContainer &GetWarning() const = 0;
-
-		bool HasError()
-		{
-			return GetError().m_ErrorType != GFX_ERROR_TYPE_NONE;
-		}
-
-		bool HasWarning()
-		{
-			return GetWarning().m_WarningType != GFX_WARNING_TYPE_NONE;
-		}
 	};
 
 	CGraphicsBackend_Threaded(TTranslateFunc &&TranslateFunc);
@@ -81,7 +71,7 @@ public:
 	bool IsIdle() const override;
 	void WaitForIdle() override;
 
-	void ProcessError();
+	void ProcessError(const SGFXErrorContainer &Error);
 
 protected:
 	void StartProcessor(ICommandProcessor *pProcessor);
@@ -199,10 +189,10 @@ public:
 	virtual ~CCommandProcessor_SDL_GL();
 	void RunBuffer(CCommandBuffer *pBuffer) override;
 
-	SGFXErrorContainer &GetError() override;
+	const SGFXErrorContainer &GetError() const override;
 	void ErroneousCleanup() override;
 
-	SGFXWarningContainer &GetWarning() override;
+	const SGFXWarningContainer &GetWarning() const override;
 
 	void HandleError();
 	void HandleWarning();