Re: JavaHL bug in native code?

On Tue, Dec 20, 2011 at 7:53 PM, Philip Martin
<philip.martin_at_wandisco.com> wrote:
>>>> 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?

No, that is why I posted this in the beginning and am confused. We do
not do anything with an array. That is why I was thinking the
exception has to be happening in the native code and somehow being
thrown back to Java.

As you know, this code also makes use of various callbacks for things
like notifications, so we are trying to see if somehow the error could
be happening there. But we do not see any usage of arrays in that
code, and when we inserted an error in that code intentionally the
runtime stack trace clearly showed where the error happens. In this
case, the stack trace seems to say the error happens in the native
code.

Late last evening, we did manage to recreate the problem using switch,
right down to the same reference to element 68. We are going to look
into it more today to see if we can narrow it down.

-- 
Thanks
Mark Phippard
http://markphip.blogspot.com/