Bug #7513: TracePoint#enable/disable should not cause error

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: 2.0.0
• ruby -v: ruby 2.0.0dev (2012-12-01 trunk 38127) [i386-mswin32_100]

[ko1 (Koichi Sasada)]

=begin
= Abstract

TracePoint#enable/disable should not cause error if it is enabled or disabled.

= Problem

The following code cause error because it calls "enable" on enabled tracepoint.

trace = TracePoint.trace{}
p trace.enabled? #=> true
trace.enable #=> `enable': trace is already enable (RuntimeError)

However, similar feature "GC.enable" and "GC.disable" don't cause error.
They only return previous status.

= Solution

TracePoint#enable and disable should align GC.enable/disable.

trace = TracePoint.trace{} # enable
trace.enable # do nothing, and return true (enabled)
trace.enable{
...
} # after block, trace is still enable

Any comments?

=end

[ko1 (Koichi Sasada)]

(2012/12/05 11:52), ko1 (Koichi Sasada) wrote:

TracePoint#enable/disable should not cause error if it is enabled or disabled.

Patch:

Index: ChangeLog

--- ChangeLog (revision 38200)
+++ ChangeLog (working copy)
@@ -1,3 +1,11 @@
+Wed Dec 5 12:45:30 2012 Koichi Sasada [e-mail]
+

• • vm_trace.c: TracePoint#enable should not cause an error
• when it is already enabled. TracePoint#disable is too.
• [Bug #7513]
• • test/ruby/test_settracefunc.rb: fix tests.

Wed Dec 5 11:42:38 2012 Koichi Sasada [e-mail]

* test/ruby/test_settracefunc.rb: disable trace.

Index: vm_trace.c

--- vm_trace.c (revision 38199)
+++ vm_trace.c (working copy)
@@ -952,17 +952,20 @@ rb_tracepoint_disable(VALUE tpval)

/*

• call-seq:

• • trace.enable -> trace

• • trace.enable -> bool
  • trace.enable { block } -> obj
  • Activates the trace

• • Will raise a RuntimeError if the trace is already activated

• • Return true if trace was enabled.
• • Return false if trace was disabled.
  • trace.enabled? #=> false

• • trace.enable #=> #TracePoint:0x007fa3fad4aaa8

• • trace.enable #=> false (previous state)

• •                  #   trace is enabled
    

  • trace.enabled? #=> true

• • trace.enable #=> RuntimeError

• • trace.enable #=> true (previous state)

• •                  #   trace is still enabled
    

  • If a block is given, the trace will only be enabled within the scope
    of the
  • block. Note: You cannot access event hooks within the block.
    @@ -986,17 +989,16 @@ static VALUE
    tracepoint_enable_m(VALUE tpval)
    {
    rb_tp_t *tp = tpptr(tpval);

• if (tp->tracing) {
• rb_raise(rb_eRuntimeError, "trace is already enable");
• }

• int previous_tracing = tp->tracing;
  rb_tracepoint_enable(tpval);
• if (rb_block_given_p()) {

• return rb_ensure(rb_yield, Qnil, rb_tracepoint_disable, tpval);

• return rb_ensure(rb_yield, Qnil,

•  	 previous_tracing ? rb_tracepoint_enable : rb_tracepoint_disable,
  

•  	 tpval);
  

  }
  else {

• return tpval;

• return previous_tracing ? Qtrue : Qfalse;
  }
  }

@@ -1007,12 +1009,13 @@ tracepoint_enable_m(VALUE tpval)
*

• Deactivates the trace

• • Will raise a RuntimeError if the trace is already deactivated

• • Return true if trace was enabled.
• • Return false if trace was disabled.
  • trace.enabled? #=> true

• • trace.disable #=> #TracePoint:0x007fa3fad4aaa8

• • trace.disable #=> false (previous status)
  • trace.enabled? #=> false

• • trace.disable #=> RuntimeError

• • trace.disable #=> false
  • If a block is given, the trace will only be disable within the scope
    of the
  • block. Note: You cannot access event hooks within the block.
    @@ -1028,25 +1031,21 @@ tracepoint_enable_m(VALUE tpval)
  • trace.enabled?
  • #=> true

• • trace.enable { p trace.lineno }
• • #=> RuntimeError: access from outside
• */
  static VALUE
  tracepoint_disable_m(VALUE tpval)
  {
  rb_tp_t *tp = tpptr(tpval);
• if (!tp->tracing) {
• rb_raise(rb_eRuntimeError, "trace is not enable");
• }

• int previous_tracing = tp->tracing;
  rb_tracepoint_disable(tpval);
• if (rb_block_given_p()) {

• return rb_ensure(rb_yield, Qnil, rb_tracepoint_enable, tpval);

• return rb_ensure(rb_yield, Qnil,

•  	 previous_tracing ? rb_tracepoint_enable : rb_tracepoint_disable,
  

•  	 tpval);
  

  }
  else {

• return tpval;

• return previous_tracing ? Qtrue : Qfalse;
  }
  }

Index: test/ruby/test_settracefunc.rb

--- test/ruby/test_settracefunc.rb (revision 38200)
+++ test/ruby/test_settracefunc.rb (working copy)
@@ -619,6 +619,16 @@ class TestSetTraceFunc < Test::Unit::Tes
}
foo
assert_equal([:foo], ary)
+

• trace = TracePoint.new{}
• begin

•  assert_equal(false, trace.enable)
  

•  assert_equal(true, trace.enable)
  

•  trace.enable{}
  

•  assert_equal(true, trace.enable)
  

• ensure

•  trace.disable
  

• end
  end

def test_tracepoint_disable

@@ -633,6 +643,14 @@ class TestSetTraceFunc < Test::Unit::Tes
foo
trace.disable
assert_equal([:foo, :foo], ary)
+

• trace = TracePoint.new{}
• trace.enable{

•  assert_equal(true, trace.disable)
  

•  assert_equal(false, trace.disable)
  

•  trace.disable{}
  

•  assert_equal(false, trace.disable)
  

• }
  end

def test_tracepoint_enabled

--
// SASADA Koichi at atdot dot net

[zzak (zzak _)]

=begin
Hi Koichi-san,

For boolean call-seq, I like: trace.enable -> true or false

Re: calling event hooks within enable/disable block.

I mean here: (({trace.enable { p trace.lineno } #=> RuntimeError: access from outside}))
=end

[ko1 (Koichi Sasada)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r38227.
Koichi, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• vm_trace.c: TracePoint#enable should not cause an error
  when it is already enabled. TracePoint#disable is too.
  [ruby-core:50561] [ruby-trunk - Bug #7513]
• test/ruby/test_settracefunc.rb: add tests.

[ko1 (Koichi Sasada)]

• Status changed from Closed to Feedback

[ko1 (Koichi Sasada)]

(2012/12/06 4:56), zzak (Zachary Scott) wrote:

For boolean call-seq, I like: trace.enable -> true or false

Okay.

Re: calling event hooks within enable/disable block.

I mean here: (({trace.enable { p trace.lineno } #=> RuntimeError: access from outside}))

Thank you. It makes sense.

I'll commit it because there are no compatible issue.
If you try and feel strange, please comment us.

--
// SASADA Koichi at atdot dot net

[zzak (zzak _)]

Thanks koichi, this is much better.

[ko1 (Koichi Sasada)]

• Status changed from Feedback to Closed