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
Received on Thu Jul 1 20:42:00 2004