FatLazy in Lift 3.5.0

[Baskaran S]

Hello Lift Authors,

I am new to Lift. Not sure I want to raise this, but I may learn something here.

In Lift 3.5.0,  FatLazy.setFrom(other) appears to access other.value in unsafe manner (not sure, maybe).

Source: https://github.com/lift/framework/blob/30152cc04153a02ed6ac397f0dbfb90aa43982df/core/util/src/main/scala/net/liftweb/util/FatLazy.scala#L80

class FatLazy[T](f: => T) {

  private var value: Box[T] = Empty

  ...

  def setFrom(other: FatLazy[T]): Unit = synchronized {

    value = other.value //original assignment

    value = Full(other.get) //proposed change

  }

  ...

}

In a general case, the original assignment may read dirty values, the proposed change may lead to deadlocks and incur runtime overheads.

-Baskaran

[Antonio Salazar Cardozo]

Def seems like a real issue. Might make more sense to just make this call set(other.get), at that point… but I think the issue is that the intent is to preserve the Empty state if present? Another option could be to mark that field volatile at least. 

[Baskaran S]

Hello Antonio,

Oh, so nice, we are rolling.

def setFrom(other: FatLazy[T]): Unit = synchronized { set(other.get) }

def setFrom(other: FatLazy[T]): Unit = set(other.get)

Then, setFrom may even give up synchronization (TBD).

I lack the knowledge about Lift's implementation architecture and its runtime architecture, so not much to contribute, so I will be a passive observer.

At present, I am taking the first steps to understand the web developers' perspective (developer facing view) of Lift's architecture. From time to time I may ask a few things "here and there" and "this and that" to understand Lift's ecosystem. Please help me out. Thank you for your support and the community support.

I am learning quite a bit from code review, tough job but liking it, looooong way to go. Good code.

-Baskaran