java - Is my ExpirableLazyObject not thread safe? Is there a race condition? -

a while back, created java class encapsulates lazy initialization lazyobect. maintains concurrency , threadsafety , provides reset() method clear value.

public final class lazyobject<t> {      private volatile t value;     private volatile boolean updated;     private final supplier<t> supplier;      private lazyobject(supplier<t> supplier) {         this.supplier = supplier;     }     public t get() {         if (!updated) {             synchronized(this) {                 if (!updated) {                     value = supplier.get();                     updated = true;                 }             }         }         return value;     }     public void reset() {         if (updated) {             synchronized(this) {                 if (updated) {                     updated = false;                     value = null;                 }             }         }     }     public static <b> lazyobject<b> forsupplier(supplier<b> supplier) {         return new lazyobject<b>(supplier);     } } 

i wanted try , take concept little farther. if lazyobject holding large object used short time (like memory-intensive hashmap), want value destroyed after period of no use. if ends being used again later, initialize again , reschedule destruction cycle.

so created expirablelazyobject.

public final class expirablelazyobject<t> {      private final lazyobject<t> value;      private final scheduledthreadpoolexecutor executor;     private final long expirationdelay;     private final timeunit timeunit;     private volatile scheduledfuture<?> scheduledremoval;      public expirablelazyobject(supplier<t> supplier, scheduledthreadpoolexecutor executor, long expirationdelay, timeunit timeunit) {          value = lazyobject.forsupplier(() -> supplier.get());         this.expirationdelay = expirationdelay;         this.executor = executor;         this.timeunit = timeunit;     }      public t get() {          if (scheduledremoval != null) {                 scheduledremoval.cancel(true);         }          t returnval = value.get();         scheduledremoval = executor.schedule(() -> value.reset(), expirationdelay, timeunit);          return returnval;     } } 

it takes supplier , needed arguments schedule destruction of value. delay destruction every time get() called. of course, designs force client handle creation , destruction of value via gc, apis manage instances internally.

the benefits can persist cached objects long enough support operation, , can lazily , regularly refresh parameters automatically.

however cannot shake feeling get() method might have race condition, cannot figure out exact reason why. don't know if need synchronized blocks or if not identifying atomicity. every synchronized block made appease concerns undermine concurrency or introduce new race condition. way can see prevent race condition (if there one) synchronize entire method. undermine concurrency. there problem here?

update has been established there race condition. think have few ideas on how fix this, i'd hear suggestions efficiently accomplish , maximize concurrency.

yes, there race condition.

t1:

    //cancel original future     if (scheduledremoval != null) {             scheduledremoval.cancel(true);     } 

t2:

    //cancel original future again      if (scheduledremoval != null) {             scheduledremoval.cancel(true);     } 

t1:

//set new future (nf) scheduledremoval = executor.schedule(() -> value.reset(), expirationdelay, timeunit);   

t2:

//set newer future (enf) scheduledremoval = executor.schedule(() -> value.reset(), expirationdelay, timeunit); 

in last step overwrite scheduledremoval new value without cancelling future. subsequent calls see enf, while nf inaccessible , uncancellable (but still active).

the easiest solution via atomicreference:

private atomicreference<scheduledfuture<?>> scheduledremoval;  public t get() {      scheduledfuture<?> next = executor.schedule(() -> value.reset(), expirationdelay, timeunit);     scheduledfuture<?> current = scheduledremoval.getandset( next );     if (current != null) {             current.cancel(true);     }      t returnval = value.get();      return returnval; } 

note can still run situation time you'd want call current.cancel(), fired. avoiding require more complicated signalling, i'm not sure worth it.