Skip to content

Commit

Permalink
Fix deadlock caused by REPL blinking
Browse files Browse the repository at this point in the history
The REPL beeping (blinking) code made the assumption that the prompt
prefix is always a String. That's not necessarily the case, and in
fact it can be an arbitrary funcion to be called when the prompt is
printed (e.g. TerminalExtensions uses this for the iTerm integration).
However, even worse, this code acquired a lock that it never released
on error, causing a deadlock. Fix the original assumption and also
add a try/catch/finally block such that future errors print the error
rather than deadlocking the terminal.

Fixes Keno/TerminalExtensions.jl#40
  • Loading branch information
Keno committed Nov 22, 2018
1 parent f1071a7 commit 038d866
Showing 1 changed file with 18 additions and 13 deletions.
31 changes: 18 additions & 13 deletions stdlib/REPL/src/LineEdit.jl
Original file line number Diff line number Diff line change
Expand Up @@ -162,21 +162,26 @@ function beep(s::PromptState, duration::Real=options(s).beep_duration,
s.beeping = min(s.beeping + duration, maxduration)
@async begin
trylock(s.refresh_lock) || return
orig_prefix = s.p.prompt_prefix
colors = Base.copymutable(colors)
use_current && push!(colors, orig_prefix)
i = 0
while s.beeping > 0.0
prefix = colors[mod1(i+=1, end)]
s.p.prompt_prefix = prefix
try
orig_prefix = s.p.prompt_prefix
colors = Base.copymutable(colors)
use_current && push!(colors, prompt_string(orig_prefix))
i = 0
while s.beeping > 0.0
prefix = colors[mod1(i+=1, end)]
s.p.prompt_prefix = prefix
refresh_multi_line(s, beeping=true)
sleep(blink)
s.beeping -= blink
end
s.p.prompt_prefix = orig_prefix
refresh_multi_line(s, beeping=true)
sleep(blink)
s.beeping -= blink
s.beeping = 0.0
catch e
Base.showerror(stdout, e, catch_backtrace())
finally
unlock(s.refresh_lock)
end
s.p.prompt_prefix = orig_prefix
refresh_multi_line(s, beeping=true)
s.beeping = 0.0
unlock(s.refresh_lock)
end
nothing
end
Expand Down

2 comments on commit 038d866

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.