mabe02 / lanterna

Java library for creating text-based GUIs
GNU Lesser General Public License v3.0
2.29k stars 243 forks source link

lanterna 3.0.0-beta 3: still has race condition / deadlock issue #295

Open mbelikov opened 7 years ago

mbelikov commented 7 years ago

S. the issue #2 The last version of the lanterna seems still to have race condition / deadlock issue at least on Windows. The deadlock is easily to reproduce, just imagine the following code (I've used scala):

object LanternaBugs {
  class AView {
    var maxVisibleCol = 50
    var maxVisibleRow = 255

    val factory: DefaultTerminalFactory = new DefaultTerminalFactory()
    var terminal = factory.createTerminalEmulator()
    //Thread.sleep(200L)

    val screen: Screen = new VirtualScreen(new TerminalScreen(terminal))
    //Thread.sleep(200L)

    terminal.addResizeListener((_: Terminal, newSize: TerminalSize) => {
      updateMaxVisibleSize(newSize)
      screen.doResizeIfNecessary()
    })

    def start() = {
      screen.startScreen()
    }

    def stop() = {
      screen.stopScreen()
    }

    def updateMaxVisibleSize(newSize: TerminalSize) = {
      maxVisibleCol = newSize.getColumns - 1
      maxVisibleRow = newSize.getRows - 1
    }

    def textCharacter(c: Char): TextCharacter = {
      new TextCharacter(c, TextColor.ANSI.DEFAULT, TextColor.ANSI.DEFAULT)
    }

    def printAt(col: Int, row: Int, msg: String): Unit = {
      msg.zipWithIndex.foreach(tuple => {
        if ((col + tuple._2) <= maxVisibleCol && row <= maxVisibleRow) {
          screen.setCharacter(col + tuple._2, row, textCharacter(tuple._1))
        }
      })
    }

    def print(col: Int, row: Int, msg: String): Unit = {
      printAt(col, row, msg)
      screen.refresh()
    }
  }

  def main(args: Array[String]): Unit = {
    val aView = new AView
    aView.start
    aView.print(2, 2, "Hello from lanterna!")

    // ...
  }
}

If I run it on my machine (i7 CPU with 4 cores) in more as 50% of runs I do get a deadlock.

The deadlock:

Found one Java-level deadlock:
=============================
"AWT-EventQueue-0":
  waiting to lock monitor 0x000000001c47b948 (object 0x000000076bc3c9c0, a com.googlecode.lanterna.screen.TerminalScreen),
  which is held by "main"
"main":
  waiting to lock monitor 0x000000001c47cf48 (object 0x000000076bb3cd28, a com.googlecode.lanterna.terminal.swing.SwingTerminalImplementation),
  which is held by "AWT-EventQueue-0"

Java stack information for the threads listed above:
===================================================
"AWT-EventQueue-0":
    at com.googlecode.lanterna.screen.TerminalScreen.doResizeIfNecessary(TerminalScreen.java:342)
    - waiting to lock <0x000000076bc3c9c0> (a com.googlecode.lanterna.screen.TerminalScreen)
    at com.googlecode.lanterna.screen.VirtualScreen.doResizeIfNecessary(VirtualScreen.java:120)
    - locked <0x000000076bc32f80> (a com.googlecode.lanterna.screen.VirtualScreen)
    at LanternaBugs$AView.$anonfun$new$1(LanternaBugs.scala:20)
    at LanternaBugs$AView$$Lambda$16/344560770.onResized(Unknown Source)
        at com.googlecode.lanterna.terminal.AbstractTerminal.onResized(AbstractTerminal.java:79)
    - locked <0x000000076bb4fce0> (a com.googlecode.lanterna.terminal.virtual.DefaultVirtualTerminal)
    at com.googlecode.lanterna.terminal.AbstractTerminal.onResized(AbstractTerminal.java:66)
    - locked <0x000000076bb4fce0> (a com.googlecode.lanterna.terminal.virtual.DefaultVirtualTerminal)
    at com.googlecode.lanterna.terminal.virtual.DefaultVirtualTerminal.setTerminalSize(DefaultVirtualTerminal.java:107)
    - locked <0x000000076bb4fce0> (a com.googlecode.lanterna.terminal.virtual.DefaultVirtualTerminal)
    at com.googlecode.lanterna.terminal.swing.GraphicalTerminalImplementation.paintComponent(GraphicalTerminalImplementation.java:266)
    - locked <0x000000076bb3cd28> (a com.googlecode.lanterna.terminal.swing.SwingTerminalImplementation)
    at com.googlecode.lanterna.terminal.swing.SwingTerminal.paintComponent(SwingTerminal.java:183)
    - locked <0x000000076b773378> (a com.googlecode.lanterna.terminal.swing.SwingTerminal)
<skipped>

"main":
    at com.googlecode.lanterna.terminal.swing.GraphicalTerminalImplementation.setCursorPosition(GraphicalTerminalImplementation.java:684)
    - waiting to lock <0x000000076bb3cd28> (a com.googlecode.lanterna.terminal.swing.SwingTerminalImplementation)
    at com.googlecode.lanterna.terminal.swing.SwingTerminal.setCursorPosition(SwingTerminal.java:206)
    at com.googlecode.lanterna.terminal.swing.SwingTerminalFrame.setCursorPosition(SwingTerminalFrame.java:205)
    at com.googlecode.lanterna.screen.TerminalScreen.startScreen(TerminalScreen.java:98)
<skipped>

Platform used:

Adding some

Thread.sleep(200L)

seems to fix the problem (e.g. after terminal = factory.createTerminalEmulator() and val screen: Screen = new VirtualScreen(new TerminalScreen(terminal))) => race condition

avl42 commented 7 years ago

Deadlock occurs, because screen has synchronized methods that call synchronized methods of Terminal, and by means of your resize listener, synchronized methods of the Terminal may now also call synchronized methods of the Screen ==> potential deadlock.

You shouldn't ever need to call screen.doResizeIfNecessary() from the Terminal's resize listener, because main thread already calls it in each iteration of the main loop.

mbelikov commented 7 years ago

Hi avl42,

sorry, didn't get your point about 'main thread already calls it in each iteration of the main loop'. Which loop in the main thread do you mean? The problem is that in my code we have differnt threads with different synchronization nesting order as far as I understand you. The proper order is then to allow calls from Screen-level to Terminal-level only and not vice-versa, right?

Ok, it is probably not a proper way to get laterna framework handling the resizes well but at least a convinient one: the screen is a super-level of the terminal but without calling of the screen.doResizeIfNecessary() the SwingTerminal under Windows doesn't repair the content after resizing (shrinking + expanding) so I just get to see the content of the last smallest terminal size. And this resize listener at the Terminal is the only place I've found where it can be catched...

How I can get then my app working properly on Windows?

Regards, mb

avl42 commented 7 years ago

You're right about asking back, and I apologize for my too cursory diagnose. (I was thinking of method "addWindowAndWait(Window)" of class MultiWindowTextGUI, but you're not using the textGui layer as far as what can be seen from the cited parts.)

Instead, the main thread is still in progress of trying to set up the Terminal from within startScreen(), when the Terminal first calls the resize listener. Have you read about the "dining philosophers" problem? If not, then google it. That's what happens here: One "dining philosopher" has the terminal lock and wants to also get the screen lock, the other one already has the screen lock and needs the terminal lock -> starvation!

The only appropriate thing to do from the resize listener is calling screen's addResizeRequest(...), and whatever thread ever calls startScreen() is also reponsible to poll doResizeIfNecessary() every once in a while.

It's just so, and you see what you get from ignoring it. And there's really nothing Lanterna could do to help you in this case.

What is your main thread doing in the "// ..." part? playing an animation? waiting for user input? then there's likely already some loop in your code. -- Nothing visible at all? Then why care about resizing the screen at all?

avl42 commented 7 years ago

by the way, if the only thing ever written to screen is the String "Hello from lanterna!" then once you let the screen shrink with the Terminal to a smaller size and grow it again, then the screen itself will have forgotten your string.