From 153e60700640e322e19deab2a36171659c4acd0c Mon Sep 17 00:00:00 2001 From: janwas Date: Thu, 18 Jan 2007 22:59:44 +0000 Subject: [PATCH] # fix in sound resource management .. as mentioned in http://www.wildfiregames.com/forum/index.php?showtopic=10719&hl= thanks to matei and philip! bonus: refs #164 tried to alleviate that by moving the OS error bit to the very end, which usually means it's beyond the end of the little window unless you're looking for it. does that help? i'll venture that the OS error *might* be useful in rare cases and it wouldn't be bad to have in there. This was SVN commit r4791. --- source/lib/debug.cpp | 62 +++++++++++++++++++------------- source/lib/res/sound/snd_mgr.cpp | 30 +++++++++++++--- 2 files changed, 62 insertions(+), 30 deletions(-) diff --git a/source/lib/debug.cpp b/source/lib/debug.cpp index 00f7a7670e..c444ea779a 100644 --- a/source/lib/debug.cpp +++ b/source/lib/debug.cpp @@ -553,54 +553,66 @@ const wchar_t* debug_error_message_build( wchar_t* buf = alloc_mem(emm->alloca_buf, emm->alloca_buf_size, emm->heap_mem, max_chars); if(!buf) return L"(insufficient memory to generate error message)"; + wchar_t* pos = buf; size_t chars_left = max_chars; int len; - char description_buf[100] = {'?'}; - LibError errno_equiv = LibError_from_errno(false); - if(errno_equiv != ERR::FAIL) // meaningful translation - error_description_r(errno_equiv, description_buf, ARRAY_SIZE(description_buf)); - - char os_error[100]; - if(sys_error_description_r(0, os_error, ARRAY_SIZE(os_error)) != INFO::OK) - strcpy_s(os_error, ARRAY_SIZE(os_error), "?"); - - static const wchar_t fmt[] = + // header + len = swprintf(pos, chars_left, L"%ls\r\n" L"Location: %hs:%d (%hs)\r\n" - L"errno = %d (%hs)\r\n" - L"OS error = %hs\r\n" L"\r\n" L"Call stack:\r\n" - L"\r\n"; - int len = swprintf(buf,max_chars,fmt, - description, - fn_only, line, func, - errno, description_buf, - os_error - ); - if(len < 0) - return L"(error while formatting error message)"; + L"\r\n", + description, fn_only, line, func); + if(len < 0) goto fail; pos += len; chars_left -= len; - // add stack trace to end of message - wchar_t* pos = buf+len; const size_t chars_left = max_chars-len; + // append stack trace if(!context) skip += 2; // skip debug_error_message_build and debug_display_error LibError ret = debug_dump_stack(pos, chars_left, skip, context); if(ret == ERR::REENTERED) { - wcscpy_s(pos, chars_left, + len = swprintf(pos, chars_left, L"(cannot start a nested stack trace; what probably happened is that " L"an debug_assert/debug_warn/CHECK_ERR fired during the current trace.)" ); + if(len < 0) goto fail; pos += len; chars_left -= len; } else if(ret != INFO::OK) { - swprintf(pos, chars_left, + char description_buf[100] = {'?'}; + len = swprintf(pos, chars_left, L"(error while dumping stack: %hs)", error_description_r(ret, description_buf, ARRAY_SIZE(description_buf)) ); + if(len < 0) goto fail; pos += len; chars_left -= len; + } + else // success + { + len = (int)wcslen(buf); + pos = buf+len; chars_left = max_chars-len; } + // append OS error (just in case it happens to be relevant - + // it's usually still set from unrelated operations) + char description_buf[100] = {'?'}; + LibError errno_equiv = LibError_from_errno(false); + if(errno_equiv != ERR::FAIL) // meaningful translation + error_description_r(errno_equiv, description_buf, ARRAY_SIZE(description_buf)); + char os_error[100]; + if(sys_error_description_r(0, os_error, ARRAY_SIZE(os_error)) != INFO::OK) + strcpy_s(os_error, ARRAY_SIZE(os_error), "?"); + len = swprintf(pos, chars_left, + L"\r\n" + L"errno = %d (%hs)\r\n" + L"OS error = %hs\r\n", + errno, description_buf, os_error + ); + if(len < 0) goto fail; pos += len; chars_left -= len; + return buf; + +fail: + return L"(error while formatting error message)"; } static ErrorReaction call_display_error(const wchar_t* text, uint flags) diff --git a/source/lib/res/sound/snd_mgr.cpp b/source/lib/res/sound/snd_mgr.cpp index ffc5e5afbd..4d5c121c00 100644 --- a/source/lib/res/sound/snd_mgr.cpp +++ b/source/lib/res/sound/snd_mgr.cpp @@ -1070,12 +1070,16 @@ if(sd->o) ogg_release(sd->o); #endif } +// note: try not call this until SndData_reload is known to have succeeded. +// came up in topic#10719, "Problem freeing sounds loaded by JavaScript". +// irrespective of the h_force_free problem documented in hsd_free_all, we +// do not want to pollute hsd_list with handles that end up being freed +// (e.g. the handle was established in preparation for loading from file, +// but that load failed). static void hsd_list_add(Handle hsd); static LibError SndData_reload(SndData * sd, const char * fn, Handle hsd) { - hsd_list_add(hsd); - // // detect sound format by checking file extension // @@ -1123,7 +1127,9 @@ static LibError SndData_reload(SndData * sd, const char * fn, Handle hsd) WARN_RETURN(ERR::NOT_SUPPORTED); RETURN_ERR(stream_open(&sd->s, fn)); + sd->is_valid = 1; + hsd_list_add(hsd); return INFO::OK; } @@ -1164,11 +1170,12 @@ else } #endif - sd->al_buf = al_buf_alloc(al_data, al_size, sd->al_fmt, sd->al_freq); - sd->is_valid = 1; - (void)file_buf_free(file); + sd->al_buf = al_buf_alloc(al_data, al_size, sd->al_fmt, sd->al_freq); + + sd->is_valid = 1; + hsd_list_add(hsd); return INFO::OK; } @@ -1280,6 +1287,19 @@ static void hsd_list_free_all() // freed (list_free_all would free the source; it then releases // its SndData reference, which closes the instance because it's // RES_UNIQUE). + // + // NB: re-initializing the sound library (e.g. after changing + // HW settings) requires all handles to be freed, even if cached. + // hence, we use h_force_free. unfortunately this causes the + // handle's tag to be ignored. it is conceivable that the wrong + // handle could be freed here. + // + // we rule this out with the following argument. either we're + // called when re-initializing sound or at exit. in the former + // case, h_force_free does check the handle type: only sounds are + // ever freed. we don't care if the wrong one is closed since all + // must be stomped upon. in the latter case, it definitely doesn't + // matter what we free. hence, no problem. } // leave its memory intact, so we don't have to reallocate it later