Bug #7513

TracePoint#enable/disable should not cause error

Added by Koichi Sasada over 1 year ago. Updated over 1 year ago.

[ruby-core:50561]
Status:Closed
Priority:Normal
Assignee:Koichi Sasada
Category:core
Target version:2.0.0
ruby -v:ruby 2.0.0dev (2012-12-01 trunk 38127) [i386-mswin32_100] Backport:

Description

=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

Associated revisions

Revision 38227
Added by Koichi Sasada over 1 year ago

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

History

#1 Updated by Koichi Sasada over 1 year ago

(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 ko1@atdot.net
+
+ * vmtrace.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 ko1@atdot.net

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

Index: vmtrace.c
===================================================================
--- vm
trace.c (revision 38199)
+++ vmtrace.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
tracepointenablem(VALUE tpval)
{
rbtpt *tp = tpptr(tpval);
-
- if (tp->tracing) {
- rbraise(rbeRuntimeError, "trace is already enable");
- }
-
+ int previoustracing = tp->tracing;
rb
tracepointenable(tpval);
+
if (rb
blockgivenp()) {
- return rbensure(rbyield, Qnil, rbtracepointdisable, tpval);
+ return rbensure(rbyield, Qnil,
+ previoustracing ? rbtracepointenable : rbtracepointdisable,
+ tpval);
}
else {
- return tpval;
+ return previous
tracing ? Qtrue : Qfalse;
}
}

@@ -1007,12 +1009,13 @@ tracepointenablem(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 @@ tracepointenablem(VALUE tpval)
* trace.enabled?
* #=> true
*
- * trace.enable { p trace.lineno }
- * #=> RuntimeError: access from outside
- *
*/
static VALUE
tracepointdisablem(VALUE tpval)
{
rbtpt *tp = tpptr(tpval);
-
- if (!tp->tracing) {
- rbraise(rbeRuntimeError, "trace is not enable");
- }
-
+ int previoustracing = tp->tracing;
rb
tracepointdisable(tpval);
+
if (rb
blockgivenp()) {
- return rbensure(rbyield, Qnil, rbtracepointenable, tpval);
+ return rbensure(rbyield, Qnil,
+ previoustracing ? rbtracepointenable : rbtracepointdisable,
+ tpval);
}
else {
- return tpval;
+ return previous
tracing ? Qtrue : Qfalse;
}
}

Index: test/ruby/testsettracefunc.rb
===================================================================
--- test/ruby/test
settracefunc.rb (revision 38200)
+++ test/ruby/testsettracefunc.rb (working copy)
@@ -619,6 +619,16 @@ class TestSetTraceFunc < Test::Unit::Tes
}
foo
assert
equal([:foo], ary)
+
+ trace = TracePoint.new{}
+ begin
+ assertequal(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
assertequal([:foo, :foo], ary)
+
+ trace = TracePoint.new{}
+ trace.enable{
+ assert
equal(true, trace.disable)
+ assertequal(false, trace.disable)
+ trace.disable{}
+ assert
equal(false, trace.disable)
+ }
end

def test_tracepoint_enabled

--
// SASADA Koichi at atdot dot net

#2 Updated by Zachary Scott over 1 year ago

=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

#3 Updated by Koichi Sasada over 1 year ago

  • 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-trunk - Bug #7513]
  • test/ruby/test_settracefunc.rb: add tests.

#4 Updated by Koichi Sasada over 1 year ago

  • Status changed from Closed to Feedback

#5 Updated by Koichi Sasada over 1 year ago

(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

#6 Updated by Zachary Scott over 1 year ago

Thanks koichi, this is much better.

#7 Updated by Koichi Sasada over 1 year ago

  • Status changed from Feedback to Closed

Also available in: Atom PDF