Skip to content

Commit

Permalink
fix dangling refs for 'new' operation
Browse files Browse the repository at this point in the history
  • Loading branch information
gewang committed Oct 27, 2023
1 parent 91852e1 commit e085638
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 4 deletions.
4 changes: 3 additions & 1 deletion VERSIONS
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ ChucK VERSIONS log
=======
- (fixed) an issue with reference count cleanup on recursive functions
that return Objects
- (added) chugins API float array access
- (fixed) an issue with reference count cleanup on 'new' operator,
e.g., (new Foo).f();
- (added, developer) chugins API float array access


1.5.1.7 (October 2023)
Expand Down
11 changes: 11 additions & 0 deletions src/core/chuck_emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3574,6 +3574,17 @@ t_CKBOOL emit_engine_emit_exp_unary( Chuck_Emitter * emit, a_Exp_Unary unary )
// instantiate object, including array
if( !emit_engine_instantiate_object( emit, t, unary->array, unary->type->ref, is_array_ref ) )
return FALSE;

// the new needs to be addref, and released (later at the end of the stmt that contains this) | 1.5.1.8
Chuck_Instr_Stmt_Start * onStack = emit->stmt_stack.size() ? emit->stmt_stack.back() : NULL;
// check it
assert( onStack != NULL );
// offset variable
t_CKUINT offset = 0;
// acquire next offset
if( !onStack->nextOffset( offset ) ) return FALSE;
// append instruction, addRef=TRUE
emit->append( new Chuck_Instr_Stmt_Remember_Object( onStack, offset, TRUE ) );
}
break;

Expand Down
5 changes: 4 additions & 1 deletion src/core/chuck_instr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4465,7 +4465,7 @@ Chuck_Object * instantiate_and_initialize_object( Chuck_Type * type, Chuck_VM_Sh

//-----------------------------------------------------------------------------
// name: instantiate_object()
// desc: ...
// desc: instantiate a object, push its pointer on reg stack
//-----------------------------------------------------------------------------
inline void instantiate_object( Chuck_VM * vm, Chuck_VM_Shred * shred,
Chuck_Type * type )
Expand Down Expand Up @@ -5805,6 +5805,9 @@ void Chuck_Instr_Stmt_Remember_Object::execute( Chuck_VM * vm, Chuck_VM_Shred *
t_CKUINT * reg_sp = (t_CKUINT *)shred->reg->sp;
Chuck_VM_Object * obj = (Chuck_VM_Object *)(*(reg_sp-1));

// add-ref for certain expressions (e.g., 'new Object;`)
if( m_addRef ) CK_SAFE_ADD_REF( obj );

// check
if( !m_stmtStart )
{
Expand Down
6 changes: 4 additions & 2 deletions src/core/chuck_instr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3408,8 +3408,8 @@ struct Chuck_Instr_Stmt_Remember_Object : public Chuck_Instr
{
public:
// constructor
Chuck_Instr_Stmt_Remember_Object( Chuck_Instr_Stmt_Start * start, t_CKUINT offset )
{ m_stmtStart = start; m_offset = offset; }
Chuck_Instr_Stmt_Remember_Object( Chuck_Instr_Stmt_Start * start, t_CKUINT offset, t_CKUINT addRef = FALSE )
{ m_stmtStart = start; m_offset = offset; m_addRef = addRef; }
// execute
virtual void execute( Chuck_VM * vm, Chuck_VM_Shred * shred );
// for printing
Expand All @@ -3420,6 +3420,8 @@ struct Chuck_Instr_Stmt_Remember_Object : public Chuck_Instr
Chuck_Instr_Stmt_Start * m_stmtStart;
// data offset
t_CKUINT m_offset;
// whether to add ref (FYI func calls internal add ref; 'new' would need this additional add-ref)
t_CKBOOL m_addRef;
};


Expand Down
2 changes: 2 additions & 0 deletions src/core/chuck_parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,8 @@ string absyn_binary2str( a_Exp_Binary binary )
case ae_op_gt:
case ae_op_le:
case ae_op_ge:
case ae_op_and:
case ae_op_or:
paren = false;
spacing = true;
break;
Expand Down
7 changes: 7 additions & 0 deletions src/core/chuck_type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3136,6 +3136,13 @@ t_CKTYPE type_engine_check_exp_unary( Chuck_Env * env, a_Exp_Unary unary )
return NULL;
}

// check if return type is an Obj | 1.5.1.8
if( isobj( env, t ) && env->stmt_stack.size() )
{
// increment # of objects in this stmt that needs release
env->stmt_stack.back()->numObjsToRelease++;
}

// return the type
return t;
break;
Expand Down
24 changes: 24 additions & 0 deletions src/test/01-Basic/230-dangle-ref-new.ck
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// verify ref counting for `new` unary operator
// NOTE as of 1.5.1.8 `new` always add-ref then release at
// end of statement, so if the ref is retained, it will be
// properly disposed by statement's end

// a class
class Foo
{
// member
5 => int x;

// print stuff
fun Foo get()
{
// <<< Machine.refcount( this ) >>>;
return this;
}
}

// should print 5
<<< (new Foo).x, "" >>>;
// should be 2 ref counts: `new` and get()
<<< Machine.refcount((new Foo).get()), "" >>>;
2 changes: 2 additions & 0 deletions src/test/01-Basic/230-dangle-ref-new.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
5
2

0 comments on commit e085638

Please sign in to comment.