From: merch-redmine@...
Date: 2021-07-03T00:26:57+00:00
Subject: [ruby-core:104478] [Ruby master Feature#16889] TracePoint.enable { ... } also activates the TracePoint for other threads, even outside the block

Issue #16889 has been updated by jeremyevans0 (Jeremy Evans).

Backport deleted (2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN)
ruby -v deleted (ruby 2.6.6p146 (2020-03-31 revision 67876) [x86_64-linux])
Tracker changed from Bug to Feature

Eregon (Benoit Daloze) wrote in #note-2:
> Maybe `enable(&block)` should behave like `enable(target: block); disable`?

I looked into this.  Implementing this fairly simple, but it breaks many tests and specs.  The reason for this is that there are many tests and specs where this is not desired behavior, because they don't want to test tracing of the block itself, but tracing during the block.

Most of the breakage is actually that calling TracePoint#enable in a way that doesn't actually add any traces raises an error.  Here's one such case, taken from `test_tracepoint_thread`:

```ruby
    events = []
    thread_self = nil
    created_thread = nil
    TracePoint.new(:thread_begin, :thread_end){|tp|
      events << [Thread.current,
                 tp.event,
                 tp.lineno,  #=> 0
                 tp.path,    #=> nil
                 tp.binding, #=> nil
                 tp.defined_class, #=> nil,
                 tp.self.class # tp.self return creating/ending thread
                 ]
    }.enable{
      created_thread = Thread.new{thread_self = self}
      created_thread.join
    }
    events.reject!{|i| i[0] != created_thread}
```

Automatically making the block the target, similar to this code, results in an error:

```ruby
    events = []
    thread_self = nil
    created_thread = nil
    block = proc{
      created_thread = Thread.new{thread_self = self}
      created_thread.join
    }
    TracePoint.new(:thread_begin, :thread_end){|tp|
      events << [Thread.current,
                 tp.event,
                 tp.lineno,  #=> 0
                 tp.path,    #=> nil
                 tp.binding, #=> nil
                 tp.defined_class, #=> nil,
                 tp.self.class # tp.self return creating/ending thread
                 ]
    }.enable(target: block, &block)
    # <internal:trace_point>:199:in `enable': can not enable any hooks (ArgumentError)
```

I'm guessing this is because :thread_begin and :thread_end events aren't triggered for code targets, unlike :line events. 

Due to this issue, I don't think it makes sense to turn `enable(&block)` into `enable(target: block); disable` by default.

However, I think there value in making it nicer to enable only the targeting of the block.  You can do this already, but it is clunky.  In your example, you would do it like this:

```ruby
threads = []
inspects = []
trace = TracePoint.new(:line) do |tp|
  threads << Thread.current
  inspects << tp.inspect
end

done = false
thread = Thread.new do
  Thread.pass until done
end

block = proc do
  line_event = true
  done = true
  sleep 1
end
trace.enable(target: block, &block)
thread.join

# Expected only within enable block (lines 14-16)
puts inspects

# Expected just 1
p threads.uniq
```

I propose we support `target: :block` to support using the block passed to `enable` as the target.  This would enable the following API:

```ruby
threads = []
inspects = []
trace = TracePoint.new(:line) do |tp|
  threads << Thread.current
  inspects << tp.inspect
end

done = false
thread = Thread.new do
  Thread.pass until done
end

trace.enable(target: :block) do
  line_event = true
  done = true
  sleep 1
end
thread.join

# Expected only within enable block (lines 14-16)
puts inspects

# Expected just 1
p threads.uniq
```

I've submitted a pull request that implements this: https://github.com/ruby/ruby/pull/4624.

----------------------------------------
Feature #16889: TracePoint.enable { ... } also activates the TracePoint for other threads, even outside the block
https://bugs.ruby-lang.org/issues/16889#change-92737

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* Assignee: ko1 (Koichi Sasada)
----------------------------------------
```ruby
threads = []
inspects = []
trace = TracePoint.new(:line) do |tp|
  threads << Thread.current
  inspects << tp.inspect
end

done = false
thread = Thread.new do
  Thread.pass until done
end

trace.enable do
  line_event = true
  done = true
  sleep 1
end
thread.join

# Expected only within enable block (lines 14-16)
puts inspects

# Expected just 1
p threads.uniq
```

Results in:
```
$ ruby tpbug.rb
ruby tpbug.rb
#<TracePoint:line@tpbug.rb:14>
#<TracePoint:line@tpbug.rb:15>
#<TracePoint:line@tpbug.rb:16>
#<TracePoint:line@tpbug.rb:10>
[#<Thread:0x00005571134e3340 run>, #<Thread:0x00005571138ac828@tpbug.rb:9 dead>]
```

But I expected:
```
#<TracePoint:line@tpbug.rb:14>
#<TracePoint:line@tpbug.rb:15>
#<TracePoint:line@tpbug.rb:16>
[#<Thread:0x00005571134e3340 run>]
```

Because the RDoc says:
```
If a block is given, the trace will only be enabled within the scope of
the block.
```

For background I'm trying to improve the TracePoint specs in ruby/spec, but they are proving quite unreliable due to this.

@ko1 Thoughts?



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>