Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POJOs can be directly mutated by developers without fiber knowing #16

Open
zeroeightysix opened this issue Aug 11, 2019 · 3 comments
Open
Labels
annotations Relates to the annotation system bug Something isn't working help wanted Extra attention is needed

Comments

@zeroeightysix
Copy link
Member

Because java optimises accesses to final fields, modifying them at runtime will result in unexpected behaviour. (and fiber shouldn't be modifying those in the first place)
This means we are forced to disallow use of final in POJOs.
However, this also means developers can directly set the values in a POJO, making it desynchronised with the IR it was merged into.
Implement a system to prevent this.

  • Have developers merge the POJO back into the IR after mutating.
  • Have the IR track POJOs and a ir.markDirty() method that checks for modifications
@zeroeightysix zeroeightysix added bug Something isn't working help wanted Extra attention is needed labels Aug 11, 2019
@NikkyAI
Copy link
Contributor

NikkyAI commented Aug 11, 2019

one option would be to create a copy of the POJO, with setters and getters (via gradle task generating code)

such wrapper objects might be possible to be generated

class PojoWrapper extends Pojo {
    private Pojo wrappedObject;

    public setX(int x) {
        // do ir notify before or after
        wrappedObject.setX(x);
    }
}

but i am not sure how to properly parse the existing POJO and turn it into AST or whatever to generate the wrapper
thats why i think a simpler representation/format that covers all you need for generating a POJO would be easier to parse by a gradle task (before any java is compiled)

this is just a example of what info would be needed, not a proposed format, it could be yaml, json or a existing format (maybe idea highlights it too)

package demo
class PojoName
  int x
  String variableName "defaultvalue"

this would generate just a POJO, no extra functions on there, maybe thats something one would want to add

i think there is no real builtin way to monitor fields
maybe InvocationHandler could be used to hook into the setter mothods of a POJO,
but from what i read you have to have a interface and can only monitor calls defined there, apparently not register them dynamically at runtime, which would probably be a bad idea anyways
and it does not work on public variables anyways

@kvverti
Copy link
Contributor

kvverti commented Apr 6, 2020

You could instead of POJOs declare configurations as interfaces with getter and setter methods. Neither option is particularly desirable tho.

@zeroeightysix zeroeightysix added the annotations Relates to the annotation system label May 3, 2020
@zeroeightysix
Copy link
Member Author

Maybe we can modify the backing Property's getValue of nodes created by the annotation system to perform a reflection call on the POJO. Basically, any time the getValue method on a node is called, it'll fetch the (potentially new) value from the POJO.

Some thoughts to consider:

  1. Does this impact performance severely enough not to be feasible? Reflection is pretty slow at times, but is it a problem in a configuration system?

  2. Change listeners will only be invoked upon a getValue call, where the backing Property suddenly 'realises' the value changed.
    This means that e.g.:

    myPojo.a = 0;
    myPojo.a = 2;
    doSomethingInvolvingA(mirrorOfA);
    mirrorOfA.getValue();

    Will only call listeners of a either somewhere in doSomethingInvolvingA (if getValue is called), or during mirrorOfA.getValue();. Is this a problem?

    1. 0 will never pass through the listeners, because the Property has never 'seen' it pass by.
    2. Listeners are delayed, but will always be called just in time before a value is fetched.
  3. What to do if a node with this kind of property is detached from the IR? How do we handle merges, etc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
annotations Relates to the annotation system bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants