[
http://issues.apache.org/jira/browse/HARMONY-100?page=comments#action_12366777
]
Tim Ellison commented on HARMONY-100:
-------------------------------------
Vladimir,
Thanks for the observation. I agree there is a problem here, but I don't agree
with your conclusion.
Looking at
http://icu.sourceforge.net/apiref/icu4c/ubidi_8h.html#a22
especially the paragraph beginning "Caution: A copy of this pointer...".
and the native code in
Java_org_apache_harmony_text_BidiWrapper_ubidi_1setPara
The Harmony code passes in the address of a local variable '_embeddingLevels'
which, as you point out will go out of scope at the end of this function
(precisely the case cautioned against). If the JNI impl is data-copying then
obviously this can lead to subsequent invalid access, and if the JNI-impl is
pinning then GC could move the byte array upon release of the data.
However, I don't see how committing the memory back to the java object helps?
The memory needs to be stable until the call to
Java_org_apache_harmony_text_BidiWrapper_ubidi_1close. Right now it is java
memory allocated in the BiDiWrapper:
private static long createUBiDi(char[] text, int textStart,
byte[] embeddings, int embStart, int paragraphLength,
int flags) {
char[] realText = null;
byte[] realEmbeddings = null;
I think the solution is to allocate this realEmbeddings memory as OSMemory in
Java and pass the address into the native call, or allocating it as VM static
memory within the native code itself, then freeing it after closing the wrapper.
> text/BidiWrapper issue?
> -----------------------
>
> Key: HARMONY-100
> URL: http://issues.apache.org/jira/browse/HARMONY-100
> Project: Harmony
> Type: Bug
> Reporter: Vladimir Gorr
>
> Let's consider the following test:
> import java.text.Bidi;
> public class Test {
> public static void main(String[] args) throws Exception {
> Bidi bd = new Bidi(new char[] { 's', 's', 's' }, 0,
> new byte[] { (byte) -7, (byte) -2, (byte) -3 },
> 0, 3, Bidi.DIRECTION_DEFAULT_LEFT_TO_RIGHT);
> System.out.println("Expected 7, real " + " " +
> bd.getLevelAt(0));
> }
> }
> In my opinion the JNI implementation of Bidi (text/BidiWrapper.c file,
> ubidi_1setPara() function) contains a potential bug, namely:
> 1. If the embeddingLevels argument is not NULL then _embeddingLevels
> variable is initialized with the JNI GetByteArrayElements() function;
> 2. ICU function (ubidi_setPara) initializes ICU inner structure and puts the
> _embeddingLevels into it;
> 3. If _embeddingLevels pointer is not NULL then the JNI
> ReleaseByteArrayElements() function (with 0 as fourth parameter) is called.
> This function releases the memory (according to JNI specification) the
> _embeddingLevels pointer refers to;
> 4. After that ICU inner structure isn't initialized properly. Call of ICU
> ubidi_getLevels() function can return incorrect values (see java test above).
> It seems the JNI_COMMIT parameter instead of "0" should be passed to the
> ReleaseByteArrayElements() to avoid this problem.
> I'd like to underline the test mentioned above works w/o any issues for
> Harmony-14 contribution (although it shouldn't sometimes IMHO).
> Therefore if there are any doubts in my argumentation this issue can be
> closed as invalid.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
