From 734d6911d8ae37d63980f98656f61d5e079fc311 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Thu, 29 Aug 2019 05:15:57 +0000 Subject: [PATCH] Do not use a WeakReference for the parent-link in HWPF-Ranges Some unit-tests show sporadic failures and it seems functionality is actually broken if the WeakReference is garbage-collected. Not sure why a WeakReference was used here anyway. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1866054 13f79535-47bb-0310-9956-ffa450edef68 --- .../src/org/apache/poi/hwpf/usermodel/Range.java | 13 ++++++------- .../org/apache/poi/hwpf/usermodel/TestBug47563.java | 9 ++++++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java index 228f2a1191..1a346fb536 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java @@ -17,7 +17,6 @@ package org.apache.poi.hwpf.usermodel; -import java.lang.ref.WeakReference; import java.util.List; import org.apache.poi.hwpf.HWPFDocument; @@ -98,7 +97,7 @@ public class Range { // TODO -instantiable superclass public static final int TYPE_UNDEFINED = 6; /** Needed so inserts and deletes will ripple up through containing Ranges */ - private final WeakReference _parent; + private final Range _parent; /** The starting character offset of this range. */ protected final int _start; @@ -167,7 +166,7 @@ public class Range { // TODO -instantiable superclass _paragraphs = _doc.getParagraphTable().getParagraphs(); _characters = _doc.getCharacterTable().getTextRuns(); _text = _doc.getText(); - _parent = new WeakReference<>(null); + _parent = null; sanityCheckStartEnd(); } @@ -190,7 +189,7 @@ public class Range { // TODO -instantiable superclass _paragraphs = parent._paragraphs; _characters = parent._characters; _text = parent._text; - _parent = new WeakReference<>(parent); + _parent = parent; sanityCheckStartEnd(); sanityCheck(); @@ -558,7 +557,7 @@ public class Range { // TODO -instantiable superclass } _text.delete( _start, _end ); - Range parent = _parent.get(); + Range parent = _parent; if ( parent != null ) { parent.adjustForInsert( -( _end - _start ) ); @@ -776,7 +775,7 @@ public class Range { // TODO -instantiable superclass } Range r = paragraph; - if (r._parent.get() != this) { + if (r._parent != this) { throw new IllegalArgumentException("This paragraph is not a child of this range instance"); } @@ -1119,7 +1118,7 @@ public class Range { // TODO -instantiable superclass _end += length; reset(); - Range parent = _parent.get(); + Range parent = _parent; if (parent != null) { parent.adjustForInsert(length); } diff --git a/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestBug47563.java b/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestBug47563.java index fc6466c45c..25c5f18294 100644 --- a/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestBug47563.java +++ b/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestBug47563.java @@ -18,10 +18,12 @@ package org.apache.poi.hwpf.usermodel; import org.apache.poi.hwpf.HWPFDocument; import org.apache.poi.hwpf.HWPFTestDataSamples; +import org.apache.poi.util.HexDump; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -29,7 +31,7 @@ import java.util.List; import static org.junit.Assert.assertTrue; /** - * Bug 47563 - Exception when working with table + * Bug 47563 - Exception when working with table */ @RunWith(Parameterized.class) public class TestBug47563 { @@ -94,11 +96,12 @@ public class TestBug47563 { } String text = range.text(); + String textBytes = HexDump.toHex(text.getBytes(StandardCharsets.UTF_8)); int mustBeAfter = 0; for (int i = 0; i < rows * columns; i++) { int next = text.indexOf(Integer.toString(i), mustBeAfter); - assertTrue("Test with " + rows + "/" + columns + ": Should not find " + i + - " but found it at " + next + " with " + mustBeAfter + " in " + text + "\n" + + assertTrue("Test with " + rows + "/" + columns + ": Should find " + i + + " but did not find it (" + next + ") with " + mustBeAfter + " in " + textBytes + "\n" + text.indexOf(Integer.toString(i), mustBeAfter), next != -1); mustBeAfter = next; -- 2.39.5