On 19 February 2016 at 20:48, Stefan Kueng <tortoisesvn_at_gmail.com> wrote:
> 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++
>>>> ]]]]
>>>>
[...]
>>> 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 ;)
SQLite is very performant when there's little concurrency (as it is
with the log caching). I wonder, what was the reason of not using it
for this purpose and re-implementing everything manually.
>
>>> 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.
>
Btw we started to convert some abort() calls in status-fetching code
to normal errors in Subversion.
>> 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!
>
It took a little bit longer than I expected but as of r27210 log cache
should gracefully handle most common database corruptions. There are
still some cases when TortoiseSVN trunk will crash with a corrupted
log cache, but TortoiseSVN 1.9.3 may also crash in these cases. I'm
going to fix these edge cases later.
I disabled asynchronous structured exception handling (SEH) in
TortoiseProc in r27211.
What do you think about backporting these changes to 1.9.x? Changes
are not small, but they fix real crashes.
--
Ivan Zhakov
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=3162819
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2016-02-25 17:05:00 CET