mirror of
				https://github.com/ruby/ruby.git
				synced 2022-11-09 12:17:21 -05:00 
			
		
		
		
	Fix nested bmethod TracePoint and memory leak
df317151a5removed the code to free rb_hook_list_t, so repeated targeting of the same bmethod started to leak the hook list. You can observe how the maximum memory use scales with input size in the following script with `/usr/bin/time -v`. ```ruby o = Object.new o.define_singleton_method(:foo) {} trace = TracePoint.new(:return) {} bmethod = o.method(:foo) ARGV.first.to_i.times { trace.enable(target:bmethod){} } 4.times {GC.start} ``` After this change the maximum doesn't grow as quickly. To plug the leak, check whether the hook list is already allocated when enabling the targeting TracePoint for the bmethod. This fix also allows multiple TracePoints to target the same bmethod, similar to other valid TracePoint targets. Finally, free the rb_hook_list_t struct when freeing the method definition it lives on. Freeing in the GC is a good way to avoid lifetime problems similar to the one fixed indf31715. [Bug #18031]
This commit is contained in:
		
							parent
							
								
									cedc36ec57
								
							
						
					
					
						commit
						e75cb61d46
					
				
				
				Notes:
				
					git
				
				2022-06-10 10:10:52 +09:00 
				
			
			
			
		
		
					 3 changed files with 28 additions and 2 deletions
				
			
		|  | @ -2364,6 +2364,28 @@ CODE | |||
|     assert_equal [:tp1, 1, 2, :tp2, 3], events | ||||
|   end | ||||
| 
 | ||||
|   def test_multiple_tracepoints_same_bmethod | ||||
|     events = [] | ||||
|     tp1 = TracePoint.new(:return) do |tp| | ||||
|       events << :tp1 | ||||
|     end | ||||
|     tp2 = TracePoint.new(:return) do |tp| | ||||
|       events << :tp2 | ||||
|     end | ||||
| 
 | ||||
|     obj = Object.new | ||||
|     obj.define_singleton_method(:foo) {} | ||||
|     bmethod = obj.method(:foo) | ||||
| 
 | ||||
|     tp1.enable(target: bmethod) do | ||||
|       tp2.enable(target: bmethod) do | ||||
|         obj.foo | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     assert_equal([:tp2, :tp1], events, '[Bug #18031]') | ||||
|   end | ||||
| 
 | ||||
|   def test_script_compiled | ||||
|     events = [] | ||||
|     tp = TracePoint.new(:script_compiled){|tp| | ||||
|  |  | |||
|  | @ -405,7 +405,9 @@ rb_method_definition_release(rb_method_definition_t *def, int complemented) | |||
| 	if (alias_count + complemented_count == 0) { | ||||
|             if (METHOD_DEBUG) fprintf(stderr, "-%p-%s:%d,%d (remove)\n", (void *)def, | ||||
|                                       rb_id2name(def->original_id), alias_count, complemented_count); | ||||
|             VM_ASSERT(def->type == VM_METHOD_TYPE_BMETHOD ? def->body.bmethod.hooks == NULL : TRUE); | ||||
|             if (def->type == VM_METHOD_TYPE_BMETHOD && def->body.bmethod.hooks) { | ||||
|                 xfree(def->body.bmethod.hooks); | ||||
|             } | ||||
| 	    xfree(def); | ||||
| 	} | ||||
| 	else { | ||||
|  |  | |||
|  | @ -1226,7 +1226,9 @@ rb_tracepoint_enable_for_target(VALUE tpval, VALUE target, VALUE target_line) | |||
|         rb_method_definition_t *def = (rb_method_definition_t *)rb_method_def(target); | ||||
|         if (def->type == VM_METHOD_TYPE_BMETHOD && | ||||
|             (tp->events & (RUBY_EVENT_CALL | RUBY_EVENT_RETURN))) { | ||||
|             if (def->body.bmethod.hooks == NULL) { | ||||
|                 def->body.bmethod.hooks = ZALLOC(rb_hook_list_t); | ||||
|             } | ||||
|             rb_hook_list_connect_tracepoint(target, def->body.bmethod.hooks, tpval, 0); | ||||
|             rb_hash_aset(tp->local_target_set, target, Qfalse); | ||||
|             target_bmethod = true; | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Alan Wu
						Alan Wu