unitsofmeasurement / indriya

JSR 385 - Reference Implementation
Other
119 stars 42 forks source link

Weird behavior (with "own" ProductUnit?) #353

Open fawidmer opened 3 years ago

fawidmer commented 3 years ago

Hello everyone

We are using indriya (v2.0.4) in a multi-thread environment. As I already pointed out a while back in https://github.com/unitsofmeasurement/indriya/pull/284, we have been facing some weird erratic behavior, which we were not able to reproduce. So we figured it might be some concurrency issue.

Unfortunately, we discovered recently, that the pull request above did not resolve our issues. Maybe, you have some ideas on how we could approach our issues or where they might come from.

Some info on what we're doing:

We have observed the following weird behavior:

Questions:

Thank you for any form of help, it is much appreciated! Fabio

keilw commented 3 years ago

@fawidmer Thanks for raising this, what keeps you from upgrading to Indriya 2.1.x? I don't think being a very small team compared to something like Spring, Jakarta EE or the OpenJDK, we can afford to backport this if a fix was found, so it'll go into whatever 2.1.x version is then the current one (right now aiming at 2.1.3) but it would not be backported to anything like 2.0.x or even 1.x because they are legacy and unsupported just like JDK 9 or 14.

We have in a class a static field which we use for all of our dimensionless quantities: public static final Unit ONE = new ProductUnit(); See the attached file: Units.txt

Are you redeclaring or using AbstractUnit.ONE? It is not so clear from the description, but above sounds like you created your own copy, and since AbstractUnit.ONE is often used internally by the RI and other modules on top of it, so cloning or "forking" that is not advisable, so is there a reason you did that?

Can we use the ProductUnit constructor like that? We are not adding the unit to any of the (hash)sets in any instance of AbstractSystemOfUnits. Is that a problem?

No, while the constructor currently is open and public you should not use it because it's entirely for use by the ONE constant.

ICU4J makes a distinciton between MeasureUnit and NoUnit in recent versions and I guess if we decided to go in a similar direction, we could (with a grace period and deprecating the existing constants) introduce a similar smaller SystemOfUnits implementations like NoUnits or NonUnits (also a bit like NonSI in si-units) which would be in the same package as ProductUnit, then the constructor (also maybe with a grace period of at least a minor version) would become package-private as it was more or less intended all the time. The two constants in AbstractQuantity for consistency sake could then also be factored into the Quantities class. As a first step, please if you created your own copy try to remove that and use AbstractUnit.ONE instead. And unless you have any strong reason against that try upgrade to Indriya 2.1.2. Even the Jackson library should at runtime work with a newer Indriya version, we have only very few cases where backward-compatibility was broken over time and this may not work, but we also plan to release new versions of the libraries during the Q2/21 Release Train.

fawidmer commented 3 years ago

Thank you very much @keilw for the detailed feedback!

import javax.measure.Unit;
import javax.measure.quantity.Volume;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import tech.units.indriya.unit.Units;
import tech.uom.lib.jackson.UnitJacksonModule;

/**
 * @author fawidmer
 */
public class Temp {

    public static void main(String[] args) throws JsonProcessingException {

        ObjectMapper mpr = new ObjectMapper();
        mpr.registerModule(new UnitJacksonModule());

        Unit<Volume> value = Units.CUBIC_METRE;
        String content = mpr.writeValueAsString(value);
        Unit readValue = mpr.readValue(content, Unit.class);

    }

}

Error trace:

Exception in thread "main" java.lang.BootstrapMethodError: java.lang.NoClassDefFoundError: tech/units/indriya/ComparableUnit
    at systems.uom.ucum.format.UCUMFormat.<init>(UCUMFormat.java:206)
    at systems.uom.ucum.format.UCUMFormat$Parsing.<init>(UCUMFormat.java:486)
    at systems.uom.ucum.format.UCUMFormat$Parsing.<clinit>(UCUMFormat.java:481)
    at systems.uom.ucum.format.UCUMFormat.getInstance(UCUMFormat.java:111)
    at tech.uom.lib.jackson.UnitJacksonModule$UnitJsonSerializer.serialize(UnitJacksonModule.java:89)
    at tech.uom.lib.jackson.UnitJacksonModule$UnitJsonSerializer.serialize(UnitJacksonModule.java:69)
    at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider._serialize(DefaultSerializerProvider.java:480)
    at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:319)
    at com.fasterxml.jackson.databind.ObjectMapper._configAndWriteValue(ObjectMapper.java:4094)
    at com.fasterxml.jackson.databind.ObjectMapper.writeValueAsString(ObjectMapper.java:3404)
    at ch.ethz.idsc.tensors.implementation.Temp.main(Temp.java:23)
Caused by: java.lang.NoClassDefFoundError: tech/units/indriya/ComparableUnit
    ... 11 more
Caused by: java.lang.ClassNotFoundException: tech.units.indriya.ComparableUnit
    at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
    at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
    ... 11 more
keilw commented 3 years ago

@fawidmer Can this also be closed similar to https://github.com/unitsofmeasurement/uom-lib/issues/68 or is there a separate issue left here?

fawidmer commented 3 years ago

Dear @keilw I'm not sure, since we still don't have any way of reproducing some of the behavior I mentioned above. It's possible that these measures fixed our issue:

I will come back to you if it persists, ok?

keilw commented 3 years ago

Ok thanks @fawidmer please verify and if it works now you can close the ticket you created yourself.