Possible memory leak in MFXValidator

[gilad4]

Describe the bug I have notice that when adding a constraint to one of the controls that consist an MFXValidator (such as: MFXTextField, etc.), may cause a memory leak. See following example:

MFXTextField textField = new MFXTextField();
textField.getValidator().constraint("Empty", textField.textProperty().isNotEmpty());

MRE (Minimal Reproducible Example)

import java.util.stream.IntStream;

import io.github.palexdev.materialfx.controls.MFXTextField;
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;

public class MfxDemo extends Application {

	public static void main(String[] args) {
		launch(args);
	}

	@Override
	public void start(Stage primaryStage) {
		Scene scene = new Scene(new StackPane(), 400, 400);
		primaryStage.setScene(scene);
		primaryStage.show();

		IntStream.range(0, 10).forEach(i -> {
			MFXTextField textField = new MFXTextField();
			textField.getValidator().constraint("Empty", textField.textProperty().isNotEmpty());
		});
		IntStream.range(0, 10).forEach(i -> {
			MFXTextField textField = new MFXTextField();
		});
	}
}

To Reproduce Steps to reproduce the behavior:

1. Run example
2. Use tool such as VisualVM to see number of MFXTextField instances
3. See that the instances that had constraint added to them, are not being GC-ed.
4. Checking GC Root shows they are held by a static WeakHashMap in 'When' class - (apparently the map key still is being referenced somewhere)

Expected behavior All instanes of MFXTextField should be GC-ed

Additional note There is a workaround that can be used:

MFXTextField textField = new MFXTextField();
Constraint constraint = Constraint.of(Severity.ERROR, "Empty", textField.textProperty().isNotEmpty());
textField.getValidator().constraint(constraint);

textField.getValidator().removeConstraint(constraint); // when no longer needed

But I would at least suggest mentioning in the relevant method description that there is a possible memory leak when using it.

[gilad4]

@palexdev

The problem here lies in the static WeakHashMap in When class. The maps values items strongly reference their own keys. Taken from WeakHashMap docs :

Implementation note: The value objects in a WeakHashMap are held by ordinary strong references. Thus care should be taken to ensure that value objects do not strongly refer to their own keys, either directly or indirectly, since that will prevent the keys from being discarded. Note that a value object may refer indirectly to its key via the WeakHashMap itself; that is, a value object may strongly refer to some other key object whose associated value object, in turn, strongly refers to the key of the first value object. If the values in the map do not rely on the map holding strong references to them, one way to deal with this is to wrap values themselves within WeakReferences before inserting, as in: m.put(key, new WeakReference(value)), and then unwrapping upon each get.

I will add a pull request for you to consider...

[palexdev]

@gilad4 thank you very much, great catch I saw the PR and I think it needs some minor tweaks but it's pretty good I'll try to review it as soon as life permits haha

[MohdBam]

@gilad4 I am a junior java developer and am interested to know how you analyzed this memory leak to know the root cause. If your time permits, I would appreciate any help from you. Thank you in advance.

[gilad4]

@MohdBam Theres a nice tool called VisualVM for monitoring Java apps. In my case I used it to find objects that should have been garbage collected but apparently were not. Digging deeper along side some elimination technics led me to the referred objects.