You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by SteveKing <st...@gmx.ch> on 2004/07/01 18:41:06 UTC
Crashes when fetching status
Hi,
For some time now I get crash reports for TortoiseSVN when fetching the
status. Since TSVN has an utility which creates a dump file in case of a
crash which then the users can send me I can fix the reason for such
crashes usually very quickly. But those crashes when fetching the status
always occur inside the Subversion library, and unfortunately is the
stacktrace of the crashdump in those cases not detailed enough.
I already mentioned these problems before on this list, but since I
could not provide a reproduction recipe...
So since I still got the problem and no reproduction recipe I decided to
look at the code myself. This is what I found:
(everything refers to the latest 1.0.5 tag, _not_ trunk since TSVN is
built against the tags)
libsvn_wc/status.c: function get_dir_status()
/* Load entries file for the directory into the requested pool. */
SVN_ERR (svn_wc_entries_read (&entries, adm_access, FALSE, subpool));
here the "entries" hash is filled. But this function looks like this:
svn_error_t *
svn_wc_entries_read (apr_hash_t **entries,
svn_wc_adm_access_t *adm_access,
svn_boolean_t show_hidden,
apr_pool_t *pool)
{
apr_hash_t *new_entries;
new_entries = svn_wc__adm_access_entries (adm_access, show_hidden, pool);
if (! new_entries)
{
SVN_ERR (read_entries (adm_access, show_hidden, pool));
new_entries = svn_wc__adm_access_entries (adm_access,
show_hidden, pool);
}
*entries = new_entries;
return SVN_NO_ERROR;
}
As you can see, if "new_entries" is NULL, then the "entries" points to a
random location! It's not initialized since "new_entries" isn't. And the
function doesn't return an error.
So if svn_wc_entries_read (or better svn_wc__adm_access_entries) fails
for some reason, the entries hash is invalid.
That invalid hash leads to a crash (or at least could!). I'm not sure if
that's the reason for a crash which happened in our office today, the
crashreport showed that it crashed on line 623 in status.c (the "entry"
in that line pointed to something invalid).
And to refresh what David Kimdon replied to my last report on the
mailing list:
<quote>
Something does appear to be wrong with that code, though I'm not sure
what (subversion/libsvn_wc/status.c:1321) :
/* See if the directory was deleted or replaced. */
dir_status = apr_hash_get (pb->statii, db->path,
APR_HASH_KEY_STRING);
if ((dir_status->repos_text_status == svn_wc_status_deleted)
|| (dir_status->repos_text_status == svn_wc_status_replaced))
was_deleted = TRUE;
/* Now do the status reporting. */
SVN_ERR (handle_statii (eb, dir_status ? dir_status->entry : NULL,
db->path, db->statii, was_deleted, TRUE,
pool));
Immediately after the call to apr_hash_get() we assume that
'dir_status' != NULL. But then when we call handle_statii() we admit
that 'dir_status' might be NULL. From what Steve described it sounds
like the first assumption is the incorrect one, but I haven't figured
out what would make 'dir_status' == NULL. . .
</quote>
This also isn't fixed yet, and I get there random crashes too, because
"dir_status" _is_ sometimes NULL! Sure, I don't know why it is NULL
there, but I get crashreports telling me that it is...
I know this isn't a big concern for the command line client, because it
doesn't happen very often, is not reproducable (at least I couldn't
reproduce it yet) and a second try usually works. But for TortoiseSVN
this is a little bit more serious since a crash in the shell extension
also crashes the explorer! And that means the whole windows desktop
shuts down.
I really hope that someone with a better insight to those functions
could take a look at this and add some more checks for NULL pointers.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Crashes when fetching status
Posted by Stefan <st...@tigris.org>.
Just got another crash report with the exact same crash location:
libsvn_wc/status.c line 623.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Crashes when fetching status
Posted by SteveKing <st...@gmx.ch>.
> > As you can see, if "new_entries" is NULL, then the "entries" points to a
> > random location!
>
> No, I don't see that. How do you reason?
Oops. Sorry. Haven't looked that closely for that.
But: the hash is then still zero (which is an error), but the return value
is SVN_NO_ERROR...
And besides my wrong analysis: I'm still getting crash reports indicating a
crash in libsvn_wc\status.c : line 623
Stefan
--
"Sie haben neue Mails!" - Die GMX Toolbar informiert Sie beim Surfen!
Jetzt aktivieren unter http://www.gmx.net/info
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Crashes when fetching status
Posted by Ben Reser <be...@reser.org>.
On Fri, Jul 02, 2004 at 09:23:47AM +0200, SteveKing wrote:
> this time, I can provide a complete callstack thanks to someone who has a
> debugger installed on their system:
>
> handle_dir_entry + 0000004A
> subversion\subversion\libsvn_wc\status.c line 623
>
> get_dir_status + 0000028B
> subversion\subversion\libsvn_wc\status.c line 777 + 00000036
>
> svn_wc_entry + 000000A6
> subversion\subversion\libsvn_wc\entries.c line 727 + 00000009
>
> close_edit + 00000110
> subversion\subversion\libsvn_wc\status.c line 1540 + 0000002B
>
> svn_client_status + 0000046A
> subversion\subversion\libsvn_client\status.c line 231 + 0000000D
>
> Hope this helps to fix it.
Looks like the return from the apr_has_get on line 773 is never being
checked. Then on line 623 we use that return as a svn_wc_entry_t and
try to retrieve the kind value of that struct.
But what's really perplexing is there is no call to:
get_dir_status in libsvn_wc/entries.c, let alone on line 727. Yes I'm
looking at 1.0.5.
It'd be nice to see the arguments to svn_client_status() and maybe a
copy of the wc, because I have no idea how this is happening. Due to
the hash entry being missing, it almost makes me think the wc is messed
up somehow. So it might be nice to know how that's happening and if it
isn't another bug...
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Crashes when fetching status
Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
SteveKing wrote:
>this time, I can provide a complete callstack thanks to someone who has a
>debugger installed on their system:
>
>
Yes, that helped. I believe the attached patch fixes the problem. The
logic in get_dir_status was borken.
/Tobias
[[[
Fix a bug in get_dir_status that cause us to call handle_dir_entry
with a null entry pointer which is not safe.
* subversion/libsvn_wc/status.c
(get_dir_status): Only call handle_dir_entry if the entry is versioned.
]]]
Re: Crashes when fetching status
Posted by SteveKing <st...@gmx.ch>.
this time, I can provide a complete callstack thanks to someone who has a
debugger installed on their system:
handle_dir_entry + 0000004A
subversion\subversion\libsvn_wc\status.c line 623
get_dir_status + 0000028B
subversion\subversion\libsvn_wc\status.c line 777 + 00000036
svn_wc_entry + 000000A6
subversion\subversion\libsvn_wc\entries.c line 727 + 00000009
close_edit + 00000110
subversion\subversion\libsvn_wc\status.c line 1540 + 0000002B
svn_client_status + 0000046A
subversion\subversion\libsvn_client\status.c line 231 + 0000000D
Hope this helps to fix it.
Stefan
--
"Sie haben neue Mails!" - Die GMX Toolbar informiert Sie beim Surfen!
Jetzt aktivieren unter http://www.gmx.net/info
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Crashes when fetching status
Posted by Greg Hudson <gh...@MIT.EDU>.
On Thu, 2004-07-01 at 14:41, SteveKing wrote:
> svn_error_t *
> svn_wc_entries_read (apr_hash_t **entries,
> svn_wc_adm_access_t *adm_access,
> svn_boolean_t show_hidden,
> apr_pool_t *pool)
> {
> apr_hash_t *new_entries;
>
> new_entries = svn_wc__adm_access_entries (adm_access, show_hidden, pool);
> if (! new_entries)
> {
> SVN_ERR (read_entries (adm_access, show_hidden, pool));
> new_entries = svn_wc__adm_access_entries (adm_access,
> show_hidden, pool);
> }
>
> *entries = new_entries;
> return SVN_NO_ERROR;
> }
>
> As you can see, if "new_entries" is NULL, then the "entries" points to a
> random location!
No, I don't see that. How do you reason?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org