On 17 February 2016 at 21:59, Stefan Küng <tortoisesvn_at_gmail.com> wrote:
> On 17.02.2016 11:57, Ivan Zhakov wrote:
>> I'm currently investigating crashes in TortoiseProc like the one in
>> drdump problem 175614 [1].
>>
>> In all cases the stacktrace looks like this:
>> [[[
>>> libapr_tsvn.dll!apr_pool_destroy(apr_pool_t * pool) Line 814 C
>> libapr_tsvn.dll!apr_pool_destroy(apr_pool_t * pool) Line 811 C
>> [...]
>> libapr_tsvn.dll!apr_pool_destroy(apr_pool_t * pool) Line 811 C
>> VCRUNTIME140.dll!_CallSettingFrame() Line 50 Unknown
>> VCRUNTIME140.dll!__FrameUnwindToState() Line 1094 C++
>> VCRUNTIME140.dll!__InternalCxxFrameHandler() Line 393 C++
>> VCRUNTIME140.dll!__CxxFrameHandler3() Line 215 C++
>> ntdll.dll!RtlpExecuteHandlerForUnwind () Unknown
>> ntdll.dll!RtlUnwindEx() Unknown
>> ntdll.dll!__C_specific_handler () Unknown
>> ntdll.dll!RtlpExecuteHandlerForException () Unknown
>> ntdll.dll!RtlDispatchException() Unknown
>> ntdll.dll!KiUserExceptionDispatch () Unknown
>> libapr_tsvn.dll!allocator_alloc(apr_allocator_t * allocator,
>> unsigned __int64 in_size) Line 333 C
>> libsvn_tsvn.dll!serf_bucket_mem_alloc() Line 216 C
>> libsvn_tsvn.dll!serf_bucket_headers_setx() Line 103 C
>> [...]
>> libsvn_tsvn.dll!svn_client_update4() Line 722 C
>> TortoiseProc.exe!SVN::Update() Line 404 C++
>> ]]]]
>>
>> As far I understand, here is what happens:
>> 1. A SEH exception happens in Subversion/serf internals (access
>> violation in APR pool allocator).
>> 2. While processing the SEH, the runtime unrolls the stack and calls a
>> destructor for SVNPool object.
>> 3. The SVNPool destructor calls apr_pool_destroy(), and that causes
>> another access violation.
>> 4. The crash handler reports the second exception instead of the original.
>>
>> I was able to reproduce this behavior in debugger. Actually, I was
>> surprised to see that the runtime unrolls the stack (and invokes
>> object destructors) in case of a SEH, but this is the documented
>> behavior of asynchronous structured exception handling (/EHa) compiler
>> option.
>>
>> My question is: what was the purpose of using asynchronous structured
>> exception handling (/EHa) in TortoiseProc? I see that this option was
>> enabled in r23749 [2].
>>
>> Currently I'm inclined to disable /EHa option in TortoiseProc and use
>> ordinary SEH processing, because:
>> 1. Microsoft doesn't recommend using this option [3]:
>> [[[
>> Specifying /EHa and trying to handle all exceptions by using
>> catch(...) can be dangerous. In most cases, asynchronous exceptions
>> are unrecoverable and should be considered fatal. Catching them and
>> proceeding can cause process corruption and lead to bugs that are hard
>> to find and fix.
>> ]]]
>>
>> 2. In most cases SEH exceptions are unrecoverable and the program is
>> in an inconsistent state, so we should not even try to releases
>> resources, etc.
>>
>> 3. Current behavior seems to be hiding real problems reported to drdump.com
>
> We use catch(...) in some places, that's why /EHa is enabled.
> Yes, it can be dangerous. But especially with the log cache it's better
> to catch the exception and then remove the corrupted cache than to crash
> over and over.
Well, I see: that current log cache uses direct access to memory
mapped file with bounds checking etc. I could review the code and add
necessary checks to convert Access Violation errors to normal C++
exceptions, but I wonder why TortoiseSVN uses custom disk storage for
log caches instead of something like SQLite? Subversion libraries
expose nice API for working with sqlite databases.
> Also, for the shell extension using catch(...) is the only way to
> prevent the desktop process from crashing - too many buggy extensions
> are out there which can interfere with TSVN, and it's always TSVN that
> gets the blame...
I'm currently focused on TortoiseProc (and maybe TSVNCache). I
understand that shell extension may require different handling and we
should try everything to avoid crashing explorer.exe.
>
> If you think we can live without that, you're welcome to try.
>
I may suggest the following:
1. I'll disable asynchronous SEH exception handling in TortoiseProc.exe
2. I'll start reviewing/rewriting log cache storage not to crash in
case of corrupted database
3. We backport change (1) to 1.9.x. After that I'll investigate
drdump crashes related to the log cache storage and convert them to
C++ exceptions.
How does it sound?
--
Ivan Zhakov
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=3161687
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2016-02-19 15:20:59 CET