Most active commenters
  • pron(3)

←back to thread

207 points mfiguiere | 21 comments | | HN request time: 2.037s | source | bottom
1. bironran ◴[] No.43539321[source]
A cursory glance at "setAccessible" usage reveals popular libraries such as serializers like gson and jaxb, class manipulation and generation like cglib, aspectj and even jdk.internal.reflect, testing frameworks and libraries including junit, mockito and other mocking libraries, lombok, groovy, spring, and the list goes on and on.

My bet is that this will be yet another "checked exception" or "module system", where many applications now need to add "--add-opens". If you'll use ANY of many of the more popular frameworks or libraries you'll end up giving this assurance away, which will make library developers not able to rely on it and we're back to square one.

replies(4): >>43539649 #>>43539702 #>>43540421 #>>43540453 #
2. PathOfEclipse ◴[] No.43539649[source]
setAccessible is also used to be able to access private fields, and not just to be able to write to final fields. Most libraries shouldn't need to set final fields, and I say this as someone who was very against when they deprecated java.lang.misc.Unsafe. I've only had to set a final field once in my career and it was related to some obscure MySql/JDBC driver bug/workaround. This particular deprecation seems very sensible to me.
replies(1): >>43539750 #
3. merb ◴[] No.43539702[source]
Yeah like the module system. Looks good on paper, is probably hard to deal with. There are still tons of popular libraries that have no module-info. Java does evolve, but the direction it does is so weird. And than the tooling is strange and it’s worse that there are basically two build tools, both with their upsides and downsides but they still feel more complicated than tools for other languages like cargo, go (if you consider that), msbuild (the modern csproj stuff/slnx)
replies(1): >>43554934 #
4. eastbound ◴[] No.43539750[source]
So how should GSON initialize an object?

The theory is, go through the constructor. However, some objects are designed to go through several steps before reaching the desired state.

If GSON must deserialize {…, state:”CONFIRMED”}, it needs to call new Transaction(account1, account2, amount), then .setState(STARTED) then .setState(PENDING) then .setState(PAID) then .setState(CONFIRMED) ? That’s the theory of the constructor and mutation methods guarding the state, so that it is physically impossible to reach a wrong state.

There is a convention that deserialization is an exception to this theory: It should be able to restore the object as-is, after for example a transfer over the wire. So it was conventionally enabled to set final variables of the object, but only at initialization and only for its own good. It was assumed that, even though GSON could reach a state that was unachievable through normal means, it was, after all, the role of the programmer to add the right annotations to avoid this.

So how do we do it now?

replies(4): >>43539839 #>>43540043 #>>43541812 #>>43543379 #
5. vips7L ◴[] No.43539839{3}[source]
Why would you use GSON for objects that go through steps of state? Why would you mark fields like State as final when it is actually mutable? This just sounds like poorly designed code.

Maybe I don't know of your use case, but GSON/Jackson/Json type classes are strictly data that should only represent the data coming over the wire. If you need to further manipulate that data it sounds like the classes have too much responsibility.

replies(1): >>43540238 #
6. steveklabnik ◴[] No.43540043{3}[source]
> So how do we do it now?

The JEP says:

> the developers of serialization libraries should serialize and deserialize objects using the sun.reflect.ReflectionFactory class, which is supported for this purpose. Its deserialization methods can mutate final fields even if called from code in modules that are not enabled for final field mutation.

I don't know enough about the details here to say if that's sufficient, but I imagine that it at least should be, or if it's not, it will be improved to the point where it can be.

replies(1): >>43541216 #
7. bdangubic ◴[] No.43540238{4}[source]
all state is immutable :) a change creates new state - which is immutable
replies(1): >>43540245 #
8. vips7L ◴[] No.43540245{5}[source]
:) no its not.
replies(1): >>43541215 #
9. pron ◴[] No.43540421[source]
We've addressed that in the JEP. Serialization libraries have a special way to circumvent this (until we get Serialization 2.0), and mocking libraries may, indeed, need to set the flag, but they're rarely used in production, so if they don't enjoy some new optimisation -- no big deal.

BTW, this JEP does not apply to setAccessible generally, as that's been restricted since JDK 16, but only to the particular (and more rare) use of setAccessible to mutate instance final fields. As the JEP says, static final fields, records' internal instance fields, and final instance fields of hidden classes cannot be mutated with that approach currently, so it's never been something that's expected to work in all cases.

replies(1): >>43540920 #
10. PaulHoule ◴[] No.43540453[source]
My impression is that this will be painful for the code I work on because the libraries you mention depend on being able to modify private and/or final fields.
replies(1): >>43540491 #
11. xxs ◴[] No.43540491[source]
private fields are no issue
12. LadyCailin ◴[] No.43540920[source]
Would be nice to have a single “--test-mode” flag that is only meant to be set when running tests, and allows for all this leniency, (add opens, etc) in a single flag.
replies(1): >>43540972 #
13. pron ◴[] No.43540972{3}[source]
We should separate the problem from the solution. The problem is that running tests may require relatively many integrity-busting flags. That is true.

There are, however, better solutions than a global test-mode flag that, invariably, will be used by some in production out of laziness, leaving no auditable record of what integrity constraints need to be violated and why. When a new team lead is appointed some years later they will have a hard time trying to reduce the entropy.

The better solutions will arrive in due course, but until then, build tools can automatically add most of the necessary flags. They should be encouraged to do that.

replies(1): >>43541135 #
14. LadyCailin ◴[] No.43541135{4}[source]
So make the flag remove some other feature, which is critical to production, like the ability to run main() or something.

On the other hand, I don’t think the solution to someone holding a shotgun to their foot and threatening to pull the trigger is to make everyone wear armored shoes. They’re already a lost cause, and there are a billion other ways they can shoot their foot off, if they are so inclined.

I agree with the principal of making it hard to screw things up assuming good faith efforts (making it hard to fall in the pit of despair), so overall I like the JEP.

replies(1): >>43541341 #
15. cesarb ◴[] No.43541216{4}[source]
> The JEP says: [...]

The JEP also says:

> The sun.reflect.ReflectionFactory class only supports deserialization of objects whose classes implement java.io.Serializable.

In my experience, most classes being deserialized by libraries like GSON do not implement Serializable. Implementing Serializable is mostly done by classes which want to be serialized and deserialized through Java's native serialization format (which is used by nothing outside Java, unlike cross-platform formats like JSON or CBOR).

16. bdangubic ◴[] No.43541215{6}[source]
if you change the state, it is not same state, it is a new state
17. pron ◴[] No.43541341{5}[source]
> On the other hand, I don’t think the solution to someone holding a shotgun to their foot and threatening to pull the trigger is to make everyone wear armored shoes.

I don't think so, either, it's just that I think there are better solutions than a test-mode flag at the level of the `java` launcher. If the mechanism that runs the tests can automatically configure the appropriate capabilities without requiring the user running the tests to do manual configuration then the problem is solved for those who just want to easily run tests just as well as a test-mode configuration.

The idea of a test-mode flag has been floated before and considered; we're not ruling it out, but if such a mode is ever added, I can't tell you now what it would mean exactly. In any event, it's better to carefully study the nature of the problem and its origins before suggesting a particular solution. As Brian Goetz likes to say, today's solutions may well become tomorrow's problems.

> They’re already a lost cause, and there are a billion other ways they can shoot their foot off, if they are so inclined.

True, but our experience shows that it's not a good idea to make the bad choice the easiest one, or people may pick it out of laziness. Let those who want to shoot themselves in the foot work for it. If nothing else, it increases the chance that they learn what their (not-entirely-trivial) configuration means, and maybe they'll realise they don't want it after all.

Someone might point out that there are still ways to do the wrong thing out of laziness by blindingly copying a configuration from StackOverflow etc., but we're not done yet.

18. twic ◴[] No.43541812{3}[source]
It strikes me that we could have a way to reflectively create an object from values for all its fields in a single step - similar to what record constructor does, but for any class (could even be Class::getCanonicalConstructor, returning a java.lang.reflect.Constructor). It would be equivalent to creating an uninitialised instance and then setting its fields one by one, but the partly-initialised object would never be visible. This should probably be restricted, because it bypasses any invariants the constructor enforces, but as you say, ultimately serialisation libraries do need to do that.
replies(1): >>43541902 #
19. recursivecaveat ◴[] No.43541902{4}[source]
I don't know if Java serialization supports this kind of thing, but if you have object A has a pointer to object B and vice-versa, there's no order to deserialize them without passing through a partially-initialized state that preserves the object identity relationship. I suppose you can't construct this kind of loopy references graph with final fields without reflection in the first place, so it's kindof chicken and egg. For the very common case of DAG-shaped data or formats that don't support references I think the one-shot internal constructor works though.
20. n_plus_1_acc ◴[] No.43543379{3}[source]
Just do it like Rust
21. gf000 ◴[] No.43554934[source]
Gradle is a general build tool, while cargo/go are only for their respective languages.

The moment you need to run some code from another language on your code to generate some other code or whatever, they break down, while Gradle can be used for pretty much anything.

In other words, cargo/go only solve the cache/parallelize/resolve task dependencies problem for "hard coded" special cases, the moment you strive away from that you are in a world of pain.