Skip to content

Commit 4e6640b

Browse files
Issue-627 Extreme memory usage with large blocks of text
Fix memory leak of ParagraphText through listeners: - Make listeners static class - Use WeakReference to refer to the ParagraphText from the listeners - Remove the listeners if the ParagraphText is garbage collected.
1 parent 17a7957 commit 4e6640b

File tree

1 file changed

+131
-41
lines changed

1 file changed

+131
-41
lines changed

Diff for: richtextfx/src/main/java/org/fxmisc/richtext/ParagraphText.java

+131-41
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.fxmisc.richtext;
22

3+
import java.lang.ref.WeakReference;
34
import java.util.Arrays;
45
import java.util.Collection;
56
import java.util.HashMap;
@@ -18,6 +19,7 @@
1819
import javafx.beans.property.ObjectProperty;
1920
import javafx.beans.property.SimpleObjectProperty;
2021
import javafx.beans.value.ChangeListener;
22+
import javafx.beans.value.ObservableValue;
2123
import javafx.collections.FXCollections;
2224
import javafx.collections.MapChangeListener;
2325
import javafx.collections.ObservableMap;
@@ -93,48 +95,11 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
9395
Val<Double> leftInset = Val.map(insetsProperty(), Insets::getLeft);
9496
Val<Double> topInset = Val.map(insetsProperty(), Insets::getTop);
9597

96-
ChangeListener<IndexRange> requestLayout1 = (obs, ov, nv) -> requestLayout();
98+
ChangeListener<IndexRange> requestLayout1 = new SelectionRangeChangeListener<>(this);
99+
selections.addListener(new SelectionsSetListener<>(leftInset, requestLayout1, topInset, this));
97100

98-
selections.addListener((MapChangeListener.Change<? extends Selection<PS, SEG, S>, ? extends SelectionPath> change) -> {
99-
if (change.wasAdded()) {
100-
SelectionPath p = change.getValueAdded();
101-
p.rangeProperty().addListener(requestLayout1);
102-
103-
p.layoutXProperty().bind(leftInset);
104-
p.layoutYProperty().bind(topInset);
105-
106-
getChildren().add(selectionShapeStartIndex, p);
107-
updateSingleSelection(p);
108-
} else if (change.wasRemoved()) {
109-
SelectionPath p = change.getValueRemoved();
110-
p.rangeProperty().removeListener(requestLayout1);
111-
112-
p.layoutXProperty().unbind();
113-
p.layoutYProperty().unbind();
114-
115-
getChildren().remove(p);
116-
}
117-
});
118-
119-
ChangeListener<Integer> requestLayout2 = (obs, ov, nv) -> requestLayout();
120-
carets.addListener((SetChangeListener.Change<? extends CaretNode> change) -> {
121-
if (change.wasAdded()) {
122-
CaretNode caret = change.getElementAdded();
123-
caret.columnPositionProperty().addListener(requestLayout2);
124-
caret.layoutXProperty().bind(leftInset);
125-
caret.layoutYProperty().bind(topInset);
126-
127-
getChildren().add(caret);
128-
updateSingleCaret(caret);
129-
} else if (change.wasRemoved()) {
130-
CaretNode caret = change.getElementRemoved();
131-
caret.columnPositionProperty().removeListener(requestLayout2);
132-
caret.layoutXProperty().unbind();
133-
caret.layoutYProperty().unbind();
134-
135-
getChildren().remove(caret);
136-
}
137-
});
101+
ChangeListener<Integer> requestLayout2 = new CaretPositionChangeListener<>(this);
102+
carets.addListener(new CaretsChangeListener<>(leftInset, requestLayout2, topInset, this));
138103

139104
// XXX: see the note at highlightTextFill
140105
// highlightTextFill.addListener(new ChangeListener<Paint>() {
@@ -437,6 +402,131 @@ protected void layoutChildren() {
437402
updateBackgroundShapes();
438403
}
439404

405+
private static final class SelectionsSetListener<PS, SEG, S> implements
406+
MapChangeListener<Selection<PS, SEG, S>, SelectionPath> {
407+
private final Val<Double> leftInset;
408+
private final ChangeListener<IndexRange> requestLayout1;
409+
private final Val<Double> topInset;
410+
private final WeakReference<ParagraphText<PS, SEG, S>> ref;
411+
412+
public SelectionsSetListener(
413+
Val<Double> leftInset,
414+
ChangeListener<IndexRange> requestLayout1,
415+
Val<Double> topInset,
416+
ParagraphText<PS, SEG, S> paragraphText) {
417+
this.leftInset = leftInset;
418+
this.requestLayout1 = requestLayout1;
419+
this.topInset = topInset;
420+
ref = new WeakReference<>(paragraphText);
421+
}
422+
423+
@Override
424+
public void onChanged(
425+
javafx.collections.MapChangeListener.Change<? extends Selection<PS, SEG, S>, ? extends SelectionPath> change) {
426+
ParagraphText<PS, SEG, S> paragraphText = ref.get();
427+
if (null == paragraphText) {
428+
change.getMap().removeListener(this);
429+
return;
430+
}
431+
432+
if (change.wasAdded()) {
433+
SelectionPath p = change.getValueAdded();
434+
p.rangeProperty().addListener(requestLayout1);
435+
436+
p.layoutXProperty().bind(leftInset);
437+
p.layoutYProperty().bind(topInset);
438+
439+
paragraphText.getChildren().add(paragraphText.selectionShapeStartIndex, p);
440+
paragraphText.updateSingleSelection(p);
441+
} else if (change.wasRemoved()) {
442+
SelectionPath p = change.getValueRemoved();
443+
p.rangeProperty().removeListener(requestLayout1);
444+
445+
p.layoutXProperty().unbind();
446+
p.layoutYProperty().unbind();
447+
448+
paragraphText.getChildren().remove(p);
449+
}
450+
}
451+
}
452+
453+
private static final class SelectionRangeChangeListener<PS, SEG, S> implements ChangeListener<IndexRange> {
454+
private final WeakReference<ParagraphText<PS, SEG, S>> ref;
455+
456+
public SelectionRangeChangeListener(ParagraphText<PS, SEG, S> paragraphText) {
457+
ref = new WeakReference<>(paragraphText);
458+
}
459+
460+
@Override
461+
public void changed(ObservableValue<? extends IndexRange> observable, IndexRange oldValue, IndexRange newValue) {
462+
if (null == ref.get()) {
463+
observable.removeListener(this);
464+
} else {
465+
ref.get().requestLayout();
466+
}
467+
}
468+
}
469+
470+
private static final class CaretPositionChangeListener<PS, SEG, S> implements ChangeListener<Integer> {
471+
private final WeakReference<ParagraphText<PS, SEG, S>> ref;
472+
473+
public CaretPositionChangeListener(ParagraphText<PS, SEG, S> paragraphText) {
474+
ref = new WeakReference<>(paragraphText);
475+
}
476+
477+
@Override
478+
public void changed(ObservableValue<? extends Integer> observable, Integer oldValue, Integer newValue) {
479+
if (null == ref.get()) {
480+
observable.removeListener(this);
481+
} else {
482+
ref.get().requestLayout();
483+
}
484+
}
485+
}
486+
487+
private static final class CaretsChangeListener<PS, SEG, S> implements SetChangeListener<CaretNode> {
488+
private final Val<Double> leftInset;
489+
private final ChangeListener<Integer> requestLayout2;
490+
private final Val<Double> topInset;
491+
private final WeakReference<ParagraphText<PS, SEG, S>> ref;
492+
493+
private CaretsChangeListener(
494+
Val<Double> leftInset,
495+
ChangeListener<Integer> requestLayout2,
496+
Val<Double> topInset,
497+
ParagraphText<PS, SEG, S> paragraphText) {
498+
ref = new WeakReference<>(paragraphText);
499+
this.leftInset = leftInset;
500+
this.requestLayout2 = requestLayout2;
501+
this.topInset = topInset;
502+
}
503+
504+
@Override
505+
public void onChanged(Change<? extends CaretNode> change) {
506+
ParagraphText<PS, SEG, S> paragraphText = ref.get();
507+
if (null == paragraphText) {
508+
change.getSet().removeListener(this);
509+
return;
510+
}
511+
if (change.wasAdded()) {
512+
CaretNode caret = change.getElementAdded();
513+
caret.columnPositionProperty().addListener(requestLayout2);
514+
caret.layoutXProperty().bind(leftInset);
515+
caret.layoutYProperty().bind(topInset);
516+
517+
paragraphText.getChildren().add(caret);
518+
paragraphText.updateSingleCaret(caret);
519+
} else if (change.wasRemoved()) {
520+
CaretNode caret = change.getElementRemoved();
521+
caret.columnPositionProperty().removeListener(requestLayout2);
522+
caret.layoutXProperty().unbind();
523+
caret.layoutYProperty().unbind();
524+
525+
paragraphText.getChildren().remove(caret);
526+
}
527+
}
528+
}
529+
440530
private static class CustomCssShapeHelper<T> {
441531

442532
private final List<Tuple2<T, IndexRange>> ranges = new LinkedList<>();

0 commit comments

Comments
 (0)