REPL: spawn prompt! to avoid blocking typing, fix pkg repl transition sequencing

[IanButterworth]

Based on #54760 (comment) by @Keno

This changes the prompt! design to not consume on the two tasks until inside the lock.

However, with this design I find that there's a 50% chance that interactive startup with -t2 will block typing while Pkg loads.

As an attempt to avoid typing blocking in this design, suggested by @vchuravy I've made repl_main run on a non-sticky task and changing any repl @async calls to @spawn to avoid thread-pinning the repl task.

But the 50% chance of blocking typing while Pkg loads remains.

It seems the repl task is still not freely switching threads.

Spawning prompt! specifically avoids typing blocking, because the general repl task is made sticky by use of _wait2 as well as the @asyncs.

[Keno]

If the actual take!ing isn't happening on the main task, then the main task needs to terminate this task while holding the lock before returning.

[IanButterworth]

You mean something like close the channel on the main task and check after wait?

        t = Threads.@spawn while true
            try
                wait(s.async_channel)
            catch
                isopen(s.async_channel) || break
                rethrow()
            end
            status = @lock l begin
                fcn = take!(s.async_channel)

[IanButterworth]

But then again, if every call to prompt! is creating this task, then we should tidy up before returning from prompt!, so it should close and reopen s.async_channel but that seems racy.

[IanButterworth]

Pushed a fix

[IanButterworth]

This should be ready for review. @Keno & @vtjnash it'd be good to make sure that you're both happy with this, and that it's not likely to resurface #54754

[Keno]

I think this is probably ok. I don't fundamentally agree that the two-mutators design is any simpler than waiting for event readiness, but at this point, I'm not seeing any obvious issues.

[IanButterworth]

@vtjnash bump for review. It'd be great to get this settled and in 1.11.0-rc1 along with the rest of this work

[IanButterworth]

I'm going to merge this as I think we can agree that it makes more sense than what's on master, and it should go in alongside the other work for the 1.11-rc

[Keno]

Started seeing this after this PR:

Stacktrace:
  [1] try_yieldto(undo::typeof(Base.ensure_rescheduled))
    @ Base ./task.jl:1114
  [2] wait()
    @ Base ./task.jl:1178
  [3] wait(c::Base.GenericCondition{ReentrantLock}; first::Bool)
    @ Base ./condition.jl:139
  [4] wait
    @ ./condition.jl:134 [inlined]
  [5] take_buffered(c::Channel{Any})
    @ Base ./channels.jl:493
  [6] take!
    @ ./channels.jl:487 [inlined]
  [7] repl_backend_loop(backend::REPL.REPLBackend, get_module::Function)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:355
  [8] start_repl_backend(backend::REPL.REPLBackend, consumer::Any; get_module::Function)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:345
  [9] run_repl(repl::REPL.AbstractREPL, consumer::Any; backend_on_current_task::Bool, backend::Any)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:501
 [10] run_repl(repl::REPL.AbstractREPL, consumer::Any)
    @ REPL ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:487
 [11] (::Base.var"#1157#1159"{Bool, Symbol, Bool})(REPL::Module)
    @ Base ./client.jl:448
 [12] #invokelatest#2
    @ ./essentials.jl:1045 [inlined]
 [13] invokelatest
    @ ./essentials.jl:1042 [inlined]
 [14] run_main_repl(interactive::Bool, quiet::Bool, banner::Symbol, history_file::Bool)
    @ Base ./client.jl:432
 [15] repl_main
    @ ./client.jl:568 [inlined]
 [16] _start()
    @ Base ./client.jl:543

    nested task error: TaskFailedException
    Stacktrace:
     [1] wait(t::Task; throw::Bool)
       @ Base ./task.jl:380
     [2] wait
       @ ./task.jl:376 [inlined]
     [3] fetch
       @ ./task.jl:542 [inlined]
     [4] prompt!(term::REPL.Terminals.TextTerminal, prompt::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
       @ REPL.LineEdit ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/LineEdit.jl:2877
     [5] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
       @ REPL.LineEdit ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/LineEdit.jl:2743
     [6] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
       @ REPL ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:1493
     [7] (::REPL.var"#75#81"{REPL.LineEditREPL, REPL.REPLBackendRef})()
       @ REPL ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:498

        nested task error: ArgumentError: IOBuffer not marked
        Stacktrace:
         [1] reset
           @ ./io.jl:1429 [inlined]
         [2] reset
           @ ./stream.jl:1489 [inlined]
         [3] peek(s::Base.TTY, ::Type{Char})
           @ Base ./stream.jl:1496
         [4] peek(t::REPL.Terminals.TTYTerminal, ::Type{Char})
           @ REPL.Terminals ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/Terminals.jl:161
         [5] (::REPL.LineEdit.var"#282#284"{REPL.Terminals.TTYTerminal, REPL.LineEdit.ModalInterface, REPL.LineEdit.MIState, ReentrantLock, REPL.LineEdit.Prompt})()
           @ REPL.LineEdit ~/julia/usr/share/julia/stdlib/v1.12/REPL/src/LineEdit.jl:2843      

[nhz2]

This seems similar to the error I was seeing here: JuliaIO/TranscodingStreams.jl#224

[IanButterworth]

Proposing reverting this #54862

[nhz2]

I think the underlying issue is that the default peek:

julia/base/io.jl

Lines 341 to 347 in 0a491e0

	function peek(s::IO, ::Type{T}) where T
	mark(s)
	try read(s, T)::T
	finally
	reset(s)
	end
	end

removes the mark on the stream, by calling reset.

So if a function calls peek after marking the stream, that mark is unexpectedly lost.

read(io, ::Char) internally uses peek(io, ::UInt8) if reading non ascii:

julia/base/io.jl

Line 969 in 0a491e0

peek(io) & 0xc0 == 0x80 || break

so depending on the IO type and data, it may or may not remove the mark from the stream.

[IanButterworth]

Ah. I can now reproduce this on master if I type an emoji into the repl.

[IanButterworth]

@nhz2 that does seem incorrect, but I've not been able to reduce it to a simple example. If you have, I think we should open an issue.