Mark Phippard <markp...@gmail.com> writes:
> On Tue, Dec 20, 2011 at 2:42 PM, Philip Martin
> <philip.mar...@wandisco.com> wrote:
>> Mark Phippard <markp...@gmail.com> writes:
>>
>>> We have had a few users report a weird error that looks like they are
>>> getting an ArrayIndexOutOfBoundsException except that it appears that
>>> it is coming out of the native code. This happens during an update
>>> operation. I am going to paste some of the native code in here, in
>>> case anyone sees anything in the array handling that looks wrong. The
>>> use of ++i instead of i++ looks odd to me, but I am not a C++
>>> programmer and the general code patterns.
>>
>> ++i and i++ are equivalent in this case, and likely to be identical
>> after optimisation.
>>
>>>
>>> const apr_array_header_t *array = targets.array(subPool);
>>> SVN_JNI_ERR(targets.error_occured(), NULL);
>>> SVN_JNI_ERR(svn_client_update4(&revs, array,
>>> revision.revision(),
>>> depth,
>>> depthIsSticky,
>>> ignoreExternals,
>>> allowUnverObstructions,
>>> TRUE /* adds_as_modification */,
>>> makeParents,
>>> ctx, subPool.getPool()),
>>> NULL);
>>>
>>> JNIEnv *env = JNIUtil::getEnv();
>>> jlongArray jrevs = env->NewLongArray(revs->nelts);
>>> if (JNIUtil::isJavaExceptionThrown())
>>> return NULL;
>>> jlong *jrevArray = env->GetLongArrayElements(jrevs, NULL);
>>> if (JNIUtil::isJavaExceptionThrown())
>>> return NULL;
>>> for (int i = 0; i < revs->nelts; ++i)
>>> {
>>> jlong rev = APR_ARRAY_IDX(revs, i, svn_revnum_t);
>>> jrevArray[i] = rev;
>>> }
>>> env->ReleaseLongArrayElements(jrevs, jrevArray, 0);
>>
>> Do we need to consider ReleaseLongArrayElements raising an exception?
>>
>> Everything else looks OK.
>>
>>> return jrevs;
>
>
> The Subclipse code always calls this API with an array of exactly one
> path. I only see two arrays in this code. The list of paths to
> update, and the return value for revisions.
I suppose you could add a check to the bindings code to test whether
revs->nelts == array->nelts == 1.
> Is there ever a scenario where passing a single path to the API should
> return an array of 68ish revisions? Our code only ever looks at the
> first revision returned since our API wrapper only accepts a single
> path.
I'm a bit confused, I thought you stated that looking at element 68
caused a ArrayIndexOutOfBoundsException? Now you say you only ever look
at element 0?
The size of the jrevs array is set by the call the NewLongArray. If
revs->nelts was 68 at that point then the Java array would have 68
elements. The element values might be junk, but they would exist so I
don't see how that would lead to ArrayIndexOutOfBoundsException.
I suppose some other thread could clear the parent of the subpool
between the call to NewLongArray and the for loop. That would allow the
memory used by revs->nelts, which is allocated from the subpool, to be
reused. Then the value of revs->nelts might be "wrong" during the for
loop and that might corrupt the heap.
Or I suppose it could be unrelated heap corruption that happens to touch
the memory used by the Java array.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
