varabyte / kotter

A declarative, Kotlin-idiomatic API for writing dynamic console applications.
Apache License 2.0
567 stars 16 forks source link

Timing issues with aside sections #86

Closed cedriessen closed 1 year ago

cedriessen commented 2 years ago

First: Great library, I like it a lot!

Describe the bug

That very simple demo below always only yields

Do something...

swallowing all the asides.

session {
  section {
    textLine("Do something...")
  }.run {
    aside { textLine(" - a") }
    aside { textLine(" - b") }
    aside { textLine(" - b") }
    aside { textLine(" - f") }
    // to be replaced
  }
}

If I replace // to be replaced at the end of the run block with something like Thread.sleep(1) (yes 1 ms is enough) or a "heavy" computation that takes some ms everything's printed out. I am suspecting a timing issue.

Do you have any thoughts?

bitspittle commented 2 years ago

Hey, I am so sorry for the long delay in replying. I missed the notification and just saw it now! Especially, I really appreciate your quick example to repro.

For now, to avoid using a sleep, I would recommend solving this using runUntilSignal:

session {
  section {
    textLine("Do something...")
  }.runUntilSignal {
    aside { textLine(" - a") }
    aside { textLine(" - b") }
    aside { textLine(" - b") }
    aside { textLine(" - f") }
    signal()
  }
}

Note that you're running into a threading / race issue. When I ran this on my machine, one or two of the asides would actually print.

Beyond that, I will investiagate. I suspect what should happen is the run block shouldn't consider itself done as long as there are any remaining aside blocks queued up for rendering. It's been a while since I looked at this code though and it might be a hairy, subtle interaction...

bitspittle commented 2 years ago

OK, looked a little more. What's happening is that a section is responsible for processing any aside blocks enqueued by the run method.

But your section is ending so fast, it shuts down and then the run block continues to add more aside blocks. (section and run are two different threads working at the same time)

I'm experimenting to see if it's trivial enough to detect, when the run method finishes, if one or more asides are still left, at which point, we kick off one more render request for the section (which should print out those remaining asides).

But so far the code is fighting me a bit (because debugging subtle threading issues is hard)...

cedriessen commented 2 years ago

Thanks for looking into it and providing me with a solution.

bitspittle commented 2 years ago

You're welcome. BTW I'm planning on looking at this again today.

After getting stuck last time, I decided to detour a little. I ended up finally starting to write unit tests (something I've been meaning to do for Kotter a long time -- I always wanted good code coverage to be a requirement to tag this library as v1.0).

What pushed me over the edge was the motivation to come back to this very bug. I'm hoping I can write a test that I can quickly run hundreds/thousands of times, to make sure things are working as expected.

bitspittle commented 1 year ago

OK, got it. It wasn't really that hard to fix once I got familiar with the code again. This was a great bug -- thanks for reporting!

I also added a unit test for this issue, and ran it a lot. It seems to be a solid fix.

This will be fixed whenever 0.9.10 (or later) goes out. The signal fix above is harmless to keep at that point, but you won't need it anymore.

cedriessen commented 1 year ago

Ok. As mentioned on the other issue I will give it a try. Thanks for investing your time helping me out with that.

bitspittle commented 1 year ago

You bet, and no rush! I just wanted you to know things were available at your convenience.

Thanks for your feedback, it helped me make Kotter better.

On Sun, Oct 9, 2022, 10:19 AM Christoph Drießen @.***> wrote:

Ok. As mentioned on the other issue I will give it a try. Thanks for investing your time helping me out with that.

— Reply to this email directly, view it on GitHub https://github.com/varabyte/kotter/issues/86#issuecomment-1272588401, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKNONAUSWWREJTA4FWH2OUDWCL5BLANCNFSM57LZ3U2Q . You are receiving this because you modified the open/close state.Message ID: @.***>

cedriessen commented 1 year ago

Just wanted to let you know: I've upgraded to 1.0.0-rc1 and things work like a charm :-)