5 ms·

A cursory glance at "setAccessible" usage reveals popular libraries such as serializers like gson and jaxb, class manipulation and generation like cglib, aspect

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.

PathOfEclipse 2y ago
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.
- eastbound 2y ago
  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?
  - vips7L 2y ago
    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.
    - bdangubic 2y ago
      all state is immutable :) a change creates new state - which is immutable
      - vips7L 2y ago
        :) no its not.
        bdangubic 2y ago
        if you change the state, it is not same state, it is a new state
  - steveklabnik 2y ago
    > 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.
    - cesarb 2y ago
      > 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).
merb 2y ago
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)
- gf000 2y ago
  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.
pron 2y ago
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.
- LadyCailin 2y ago
  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.
  - pron 2y ago
    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.
    - LadyCailin 2y ago
      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.
PaulHoule 2y ago
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.
- xxs 2y ago
  private fields are no issue

PathOfEclipse 2y ago

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.

eastbound 2y ago
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?
- vips7L 2y ago
  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.
  - bdangubic 2y ago
    all state is immutable :) a change creates new state - which is immutable
    - vips7L 2y ago
      :) no its not.
      - bdangubic 2y ago
        if you change the state, it is not same state, it is a new state
- steveklabnik 2y ago
  > 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.
  - cesarb 2y ago
    > 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).

merb 2y ago

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)

gf000 2y ago
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.

pron 2y ago

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.

LadyCailin 2y ago
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.
- pron 2y ago
  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.
  - LadyCailin 2y ago
    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.

PaulHoule 2y ago

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.

xxs 2y ago
private fields are no issue