Project

General

Profile

Actions

Bug #18810

open

Make `Kernel#p` interruptable.

Added by ioquatix (Samuel Williams) 8 months ago. Updated 6 months ago.

Status:
Open
Priority:
Normal
Target version:
-
[ruby-core:108728]

Description

While figuring out https://bugs.ruby-lang.org/issues/18465 I found a test which fails when rb_io_flush becomes blocking.: https://github.com/ruby/ruby/commit/fe6b2e20e9f17ed2c2900aa72994e075ffdc7124

It seems unusual to me that Kernel#p is uninterruptible (unique among all Ruby methods). I'd like to make Kernel#p interruptible.

Updated by mame (Yusuke Endoh) 8 months ago

Since Kernel#p is a method for debugging, I think this spec would be useful. If it is made interruptable, it will be difficult to use Kernel#p in a block of Thread.handle_interrupt(TimeoutError => :on_blocking).

Updated by mame (Yusuke Endoh) 6 months ago

We discussed this issue at the dev meeting. We will add a document to Kernel#p so that it is uninterruptible and for debugging purpose.

Updated by ioquatix (Samuel Williams) 6 months ago

@mame (Yusuke Endoh) I understand the discussion and I'm okay with the outcome, but I still don't understand why being uninterruptible matters in practice. I'm still a little concerned this can hang the interpreter, but I don't know for sure - because remember the internal write function can call the fiber scheduler. So it can potentially execute a lot of Ruby code in an uninterruptible way which is very unique - it is the only method which behaves like this.

Updated by Eregon (Benoit Daloze) 6 months ago

mame (Yusuke Endoh) wrote in #note-2:

Since Kernel#p is a method for debugging, I think this spec would be useful. If it is made interruptable, it will be difficult to use Kernel#p in a block of Thread.handle_interrupt(TimeoutError => :on_blocking).

You mean if p is used inside a Timeout.timeout block?
And you'd worry the object would be printed but no newline, or the write would be interrupted in the middle and have written some partial output (is that even possible when writing to a terminal given write is atomic in at least that case?)?
If it's about the newline, that would be solved by p appending "\n" to the string and only doing a single write, everything is a million times simpler that way.

I think this is a very rare case and any developer must expect this or far worse effects of asynchronous exceptions when using Timeout.timeout/Thread#raise.

We discussed this issue at the dev meeting. We will add a document to Kernel#p so that it is uninterruptible and for debugging purpose.

IMHO this should not be part of specification, but an implementation choice/detail.
So I'd suggest to write it like "Kernel#p is uninterruptible on CRuby to avoid being interrupted by Timeout.timeout" or so in the docs.

Updated by mame (Yusuke Endoh) 6 months ago

My concern is that inserting p(...) changes a program behavior unintentionally (except that the p writes something to stdout, of course).

When debugging, we often insert p(...) and run the program to reproduce the bug being debugged. It would be annoying if the bug becomes unreproducible because the insertion of p(...) changes the behavior of the program. I don't want to see something like the "bug not reproduced on the debugger" problem.

Updated by ioquatix (Samuel Williams) 6 months ago

My concern is that inserting p(...) changes a program behavior unintentionally (except that the p writes something to stdout, of course).

There are so many ways it can do this. If the fiber scheduler is active it is a totally different code path. What about bufferred output (common on stdout).

What you really want is a blocking write to stderr, e.g. fprintf(stderr, ...). This has very few side effects.

Updated by Eregon (Benoit Daloze) 6 months ago

@mame (Yusuke Endoh) Could you give an example in Ruby code?
I don't understand your concern, of course p changes behavior because it calls inspect (which can do anything) and prints, but that seems fully expected.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0