Ran into this, is there any reason why kotlin bytecode ignores synchronization primitive for get() ...

# announcements

[Sam]

Ran into this, is there any reason why kotlin bytecode ignores synchronization primitive for get()

var data = false
    @Synchronized get
    @Synchronized set(value) { field = !value }

fun main() {
    println("Value $data")
}

The only way to get bytecode to call getData() is by having this redundant get (which is prompted as redundant getter in IDE)

@Synchronized get() = field

[nanodeath]

can you share more? bytecode looks synchronized to me

[nanodeath]

public final static synchronized getData()Z
   L0
    LINENUMBER 1 L0
    GETSTATIC SyncKt.data : Z
    IRETURN
   L1
    MAXSTACK = 1
    MAXLOCALS = 0

[Sam]

getData() is defined as synchronized but the call site doesn't invoke getData(), instead it uses data directly

[nanodeath]

GETSTATIC SyncKt.data : Z

this? .....yeah, that seems to corroborate that

[Sam]

yup, if you decompile that,

private static boolean data;

   public static final synchronized boolean getData() {
      return data;
   }

   public static final synchronized void setData(boolean value) {
      data = !value;
   }

   public static final void main() {
      String var0 = "Value " + data;
      System.out.println(var0);
   }

[nanodeath]

well, I've been told before that the decompiler is approximate, but in this case they do seem to be in agreement

[nanodeath]

(for example, the bytecode creates a StringBuilder)

[nanodeath]

which version of Kt is this?

[Sam]

1.4.0

[nanodeath]

repros for me on 1.3.72 also. sounds like a bug worth reporting.

[Sam]

Sound good

[nanodeath]

I think the YouTrack search might be having a bad time right now...some of my search queries are taking >30 seconds

[Sam]

yup likewise

[nanodeath]

oh well. good luck 😛 link back for us when you find something or report a new issue 😄

[Sam]

Sure

[araqnid]

field = !value ? That sounds like a recipe for confusion.

[nanodeath]

I'm assuming it's just an example

[Sam]

@araqnid yeah foobar, just to get around lint warning

[Sam]

https://youtrack.jetbrains.com/issue/KT-41323

[araqnid]

If you really just want to wrap synchronization around that field access, you could do this of course:

private var privateData = false
    get() = synchronized(this) { field }
    set(value) { synchronized(this) { field = value } }

With some added fun if you want this for a top-level property (maybe lock some class object similar to what static synchronized in Java does)
Or even just create a delegate for applying synchronization a la private var privateData by synchronizeOn(this) { false }

[araqnid]

But yes, the current situation where the @Synchronized annotation is ignored here seems wrong

[Sam]

yeah, Delegates works better. This was just to confirm and report the issue.

[Kroppeb]

What is the @Syncronised supposed to do here

[Kroppeb]

Standard getters get converted into standard field accessed by the JVM. Idk if this prevents that but I don't see why reading has to be synchronised?

[nanodeath]

in case something is setting it at the same time

[Kroppeb]

I don't think this will do anything about that? I thought this just prevents two threads from reading the field at the same time, which should be just fine.

[nanodeath]

if you synchronized on the getters and setters in Java it would though

[Kroppeb]

Oh, my bad, I thought synchronised was on a method basis instead of working for the full object. But shouldn't you make the variable volatile then too?

[nanodeath]

@Synchronized adds "synchronization barriers" that cause caches to be read/flushed upon entering/exiting them, IIRC, so adding volatile would be redundant.

[nanodeath]

though tbh this isn't my strong suit and I'm not looking in the right places for documentation apparently