On 19.02.2016 14:20, Ivan Zhakov wrote:
> 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.
the log cache is optimized for speed. For example, you can show the log
for the whole KDE or Apache repository (including all subproject). Then
show the log again and you get *all* entries within about two/three seconds.
But for more details, you have to ask Stefan Fuhrmann ;)
>> 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.
Yes. At least for all the status-fetching code.
> 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?
That sounds good. If you have the time to do it, then please go ahead!
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest interface to (Sub)version control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=3161773
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2016-02-19 18:48:44 CET