Re: svn commit: r1700799 - in /subversion/trunk/subversion: include/svn_io.h libsvn_subr/stream.c svnadmin/svnadmin.c svnfsfs/load-index-cmd.c tests/libsvn_subr/stream-test.c

[Evgeny Kotkov]

Stefan Fuhrmann  writes:

>> The original commit begins using svn_stream_wrap_buffered_read() during
>> svnadmin load-revprops and svnfsfs load-index.  This patch, however, does
>> something entirely different and adds buffering to *every* stdin.
>
> Sorry for the confusion. I did not intend to actually change the
> implementation of svn_stream_for_stdin this way but simply tried to
> demonstrate a problem with APR buffer reads for "streamy" file handles.

Thank you for the explanation.

> The underlying problem is still present: If stdin can't deliver data fast
> enough (e.g. some hick-up / long latency on the producer side of a dump |
> load), a buffered APR file will error out while a non-buffered one will
> simply wait & retry.
>
> However, I have yet to try and provoke the error specifically for the
> dump | load scenario.

I think that this problem is nonexistent.

A program doesn't need to handle EAGAIN during read() unless it puts the file
into the non-blocking mode with O_NONBLOCK [1].  We don't do that for STDIN,
and, irrespectively of what happens with the data on the other side of the
pipe, read() is going to block until the requested data is available.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/read.html


Regards,
Evgeny Kotkov

Re: JavaHL: Hard crash in libsvn_subr-1.dll

[Thomas Singer]

What further information do you need for this crash?

--
Best regards,
Thomas Singer
=
syntevo GmbH
http://www.syntevo.com
http://www.syntevo.com/blog


On 08.09.2015 09:06, Thomas Singer wrote:

We've got the attached bug report where libsvn_subr-1.dll crashes.
Unfortunately, I have no idea how to reproduce.

--
Best regards,
Thomas Singer
=
syntevo GmbH
http://www.syntevo.com
http://www.syntevo.com/blog

Crash in calculate_repos_relpath()

[Ivan Zhakov]

I was looking to the most reported TortoiseSVN crash reports and found
many crash in libsvn_wc/update_editor.c:551 in function
calculate_repos_relpath() due calling to svn_relpath_join() with BASE
== NULL:
[[[
  if (old_repos_relpath == NULL)
{
  SVN_ERR_ASSERT(pb != NULL);
  *new_repos_relpath = svn_relpath_join(pb->new_repos_relpath, name,
result_pool);
}
]]]

Full stack trace:
 libsvn_tsvn32.dll!svn_relpath_join(const char * base, const char
* component, apr_pool_t * pool) Line 1175C
>libsvn_tsvn32.dll!calculate_repos_relpath(const char * * 
> new_repos_relpath, const char * local_abspath, const char * 
> old_repos_relpath, edit_baton * eb, dir_baton * pb, apr_pool_t * result_pool, 
> apr_pool_t *) Line 551C
 libsvn_tsvn32.dll!add_file(const char * path, void *
parent_baton, const char * copyfrom_path, long copyfrom_rev,
apr_pool_t * pool, void * * file_baton) Line 3069C
 libsvn_tsvn32.dll!add_file(const char * path, void *
parent_baton, const char * copyfrom_path, long copyfrom_revision,
apr_pool_t * pool, void * * file_baton) Line 167C
 libsvn_tsvn32.dll!ensure_file_opened(file_baton_t * file,
apr_pool_t * scratch_pool) Line 915C
 libsvn_tsvn32.dll!update_closed(svn_ra_serf__xml_estate_t * xes,
void * baton, int leaving_state, const svn_string_t * cdata,
apr_hash_t * attrs, apr_pool_t * scratch_pool) Line 1902C
 libsvn_tsvn32.dll!xml_cb_end(svn_ra_serf__xml_context_t * xmlctx,
const char *) Line 832C
 libsvn_tsvn32.dll!expat_end(void * userData, const char *
raw_name) Line 965C


I've full dump with all process memory, so local variables contain real values.

I see the following relevant changes in calculate_repos_relpath():
[[[
Author: rhuijben
Date: Sun Dec 15 00:41:10 2013
New Revision: 1550988

URL: http://svn.apache.org/r1550988
Log:
Following up on r1550986 and r1550985, update arguments of the repos relpath
calculation function to avoid a db query for every normal node touched
during update.
]]

[[[
Author: rhuijben
Date: Mon Mar 31 10:58:19 2014
New Revision: 1583294

URL: http://svn.apache.org/r1583294
Log:
Make the add_directory and add_file code in the update editor more similar and
thereby avoid a few unneeded db operations during a clean checkout.
]]]


Bert, do you have any ideas why PB->NEW_REPOS_RELPATH could be NULL?

-- 
Ivan Zhakov

Re: JavaHL: Hard crash in libsvn_subr-1.dll

[Ivan Zhakov]

On 9 September 2015 at 16:58, Thomas Singer  wrote:
> What further information do you need for this crash?
>
Stack trace with symbols information at least.


-- 
Ivan Zhakov

Re: svn commit: r1658848 - in /subversion/trunk/subversion: libsvn_wc/token-map.h libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db_private.h libsvn_wc/wc_db_update_move.c tests/cmdline/move_

[Ivan Zhakov]

On 11 February 2015 at 03:42,   wrote:
> Author: rhuijben
> Date: Wed Feb 11 00:42:01 2015
> New Revision: 1658848
>
> URL: http://svn.apache.org/r1658848
> Log:
> Make the (non recursive) revert db operation properly report tree conflicts
> that it creates by both fixing what is stored in the tree conflict and by
> properly creating notifications.
>
> Note that a recursive revert wouldn't encounter this problem as it
> would just break the moves.
>
Hi Bert,

It seems this commit introduces potential use of uninitialized
variable CONFLICT in subversion/libsvn_wc/wc_db.c:op_revert_txn(). See
below.

> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1658848=1658847=1658848=diff
> ==
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Feb 11 00:42:01 2015
> @@ -6738,6 +6738,8 @@ op_revert_txn(void *baton,
>svn_boolean_t moved_here;
>int affected_rows;
>const char *moved_to;
> +  int op_depth_increased = 0;
> +  svn_skel_t *conflict;
CONFLICT local variable will be uninitialized if MOVED_TO is non-zero.

>
>/* ### Similar structure to op_revert_recursive_txn, should they be
>   combined? */
> @@ -6794,45 +6796,12 @@ op_revert_txn(void *baton,
>  }
>else
>  {
> -  svn_skel_t *conflict;
> -
>SVN_ERR(svn_wc__db_read_conflict_internal(, wcroot,
>  local_relpath,
>  scratch_pool, scratch_pool));
> -  if (conflict)
> -{
> -  svn_wc_operation_t operation;
> -  svn_boolean_t tree_conflicted;
> -
> -  SVN_ERR(svn_wc__conflict_read_info(, NULL, NULL, NULL,
> - _conflicted,
> - db, wcroot->abspath,
> - conflict,
> - scratch_pool, scratch_pool));
> -  if (tree_conflicted
> -  && (operation == svn_wc_operation_update
> -  || operation == svn_wc_operation_switch))
> -{
> -  svn_wc_conflict_reason_t reason;
> -  svn_wc_conflict_action_t action;
> -
> -  SVN_ERR(svn_wc__conflict_read_tree_conflict(, ,
> -  NULL,
> -  db, 
> wcroot->abspath,
> -  conflict,
> -  scratch_pool,
> -  scratch_pool));
> -
> -  if (reason == svn_wc_conflict_reason_deleted)
> -SVN_ERR(svn_wc__db_resolve_delete_raise_moved_away(
> -  db, svn_dirent_join(wcroot->abspath, local_relpath,
> -  scratch_pool),
> -  NULL, NULL /* ### How do we notify this? */,
> -  scratch_pool));
> -}
> -}
>  }
>
> +
>if (op_depth > 0 && op_depth == relpath_depth(local_relpath))
>  {
>/* Can't do non-recursive revert if children exist */
> @@ -6857,7 +6826,7 @@ op_revert_txn(void *baton,
>SVN_ERR(svn_sqlite__bindf(stmt, "isd", wcroot->wc_id,
>  local_relpath,
>  op_depth));
> -  SVN_ERR(svn_sqlite__step_done(stmt));
> +  SVN_ERR(svn_sqlite__update(_depth_increased, stmt));
>
>SVN_ERR(svn_sqlite__get_statement(, wcroot->sdb,
>  STMT_DELETE_WORKING_NODE));
> @@ -6875,6 +6844,54 @@ op_revert_txn(void *baton,
>  SVN_ERR(clear_moved_to(wcroot, local_relpath, scratch_pool));
>  }
>
> +  if (op_depth_increased && conflict)
> +{
> +  svn_wc_operation_t operation;
> +  svn_boolean_t tree_conflicted;
> +  const apr_array_header_t *locations;
> +
> +  SVN_ERR(svn_wc__conflict_read_info(, , NULL, NULL,
> +  _conflicted,
> +  db, wcroot->abspath,
> +  conflict,
> +  scratch_pool, scratch_pool));
And then used here.

I'm not expert in this area, but may you know how to reproduce and fix
this problem because I see several TortoiseSVN crashes [1] due access
to uninitialized variable CONFLICT. Call stack:
[[[
 libsvn_tsvn.dll!svn_wc__conflict_read_info(svn_wc_operation_t *
operation, const apr_array_header_t * * locations, int *
text_conflicted, int * prop_conflicted, int * tree_conflicted,
svn_wc__db_t * conflict_skel, const char