Project

General

Profile

Feature #13483

TracePoint#enable with block for thread-local trace

Added by ko1 (Koichi Sasada) over 2 years ago. Updated about 1 year ago.

Status:
Rejected
Priority:
Normal
Target version:
[ruby-core:80791]

Description

Summary

TracePoint#enable with block should enable thread-local trace.

Current behavior

TracePoint#enable enables TracePoint for all of threads, even if it called with do...end blcok.

t1 = Thread.new{
  loop{
    x = 1
  }
}
th = nil
trace = TracePoint.new(:line){|tp|
  if th != Thread.current
    p th = Thread.current
  end
}

trace.enable do
  loop{
    a = 1
    b = 2
  }
end

This program shows both main thread and thread t1 hooked by line events.

Problem

However, usually trace.enable do ... end imply the programmer want to enable hooks only for this block, not for other threads.
For example, Ruby's test for TracePoint skips hooks on other threads.
https://github.com/ruby/ruby/blob/trunk/test/ruby/test_settracefunc.rb#L620

Proposal

TracePoint#enable with block should enable thread-local trace.
I believe proposed behavior is easy to use.

Consideration

(1) It breaks backward compatibility. Is it acceptable?
(2) What happen on created threads? Should inherit thread-local hooks or ignore them?

I want to ask users of TracePoint.

Thanks,
Koichi

History

#1

Updated by shyouhei (Shyouhei Urabe) over 2 years ago

  • Status changed from Open to Assigned
#2

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Description updated (diff)

Updated by ko1 (Koichi Sasada) over 2 years ago

Matz approved it.

Updated by ko1 (Koichi Sasada) over 2 years ago

After consideration, I found several problems.

There is a implicit expectation that we can emulate block accept call with begin/ensure like:

open do
  ...
end
#=> same as:
begin
  open
  yield
ensure
  close
end

However, this proposal breaks this expectation. So I reject this ticket.

I try to consider to introduce how to filter the probes, like:

trace = TracePoint.new(:line, thread: Thread.current){ ... }
trace.enable #=> only enable on the current thread.

trace = TracePoint.new(:line, file: __FILE__){ ... }
trace.enable #=> only enable on this file.

trace = TracePoint.new(:line, method: method(:foo)){ ... }
trace.enable #=> only enable on `foo` method.

Thanks,
Koichi

#5

Updated by ko1 (Koichi Sasada) over 2 years ago

  • Status changed from Assigned to Rejected

Updated by Eregon (Benoit Daloze) over 2 years ago

ko1 (Koichi Sasada) wrote:

However, this proposal breaks this expectation.

Could you explain it?

Is it because trace.enable { code } does not behave like
begin; trace.enable; code; ensure; trace.disable; end ?

If so, I think this problem could be avoided by just changing the name to imply "thread-local",
such as trace.enable_for_current_thread { code } or
trace.enable_in_block { code }.

Updated by ko1 (Koichi Sasada) over 2 years ago

On 2017/05/27 18:49, eregontp@gmail.com wrote:

However, this proposal breaks this expectation.
Could you explain it?

Is it because trace.enable { code } does not behave like
begin; trace.enable; code; ensure; trace.disable; end ?

Yes.

If so, I think this problem could be avoided by just changing the name to imply "thread-local",
such as trace.enable_for_current_thread { code } or
trace.enable_in_block { code }.

Yes.

This is what

I try to consider to introduce how to filter the probes, like:

Considerations about introducing "thread-lcoal" enable:

(1) POSITIVE: because it may be common use case to enable.
(2) NEGATIVE:
(2-1) because enable_xxx seems verbose.
(2-2) because we will want to introduce similar method to
limit file name or method name, like enable_file do ... end
(this is why I proposed keyword arg)

--
// SASADA Koichi at atdot dot net

Updated by ioquatix (Samuel Williams) about 1 year ago

ko1 (Koichi Sasada) Was there an update to this proposal?

Also available in: Atom PDF