Skip to content
This repository was archived by the owner on Aug 31, 2021. It is now read-only.

Commit 4c0b9ca

Browse files
livecodealilivecodeali
authored
Merge pull request #6954 from runrevmark/bugfix-16947
[[ Bug 16947 ]] Fix memory leak when making recursive calls with refs
2 parents 644de0d + 4868db6 commit 4c0b9ca

File tree

20 files changed

+541
-298
lines changed

20 files changed

+541
-298
lines changed

‎docs/notes/bugfix-16947.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Fix memory leak when calling self-recursive handlers having reference parameters
2+

‎engine/src/cmds.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -997,7 +997,13 @@ class MCDispatchCmd: public MCStatement
997997
MCExpression *message;
998998
MCChunk *target;
999999
MCParameter *params;
1000-
bool is_function;
1000+
struct
1001+
{
1002+
/* The container count is the number of containers needed to execute
1003+
* the command. It is calculated after parsing the node. */
1004+
unsigned container_count : 16;
1005+
bool is_function : 1;
1006+
};
10011007

10021008
public:
10031009
MCDispatchCmd(void)
@@ -1006,6 +1012,7 @@ class MCDispatchCmd: public MCStatement
10061012
target = NULL;
10071013
params = NULL;
10081014
is_function = false;
1015+
container_count = 0;
10091016
}
10101017
~MCDispatchCmd(void);
10111018

‎engine/src/cmdse.cpp‎

Lines changed: 36 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,14 @@ Parse_stat MCDispatchCmd::parse(MCScriptPoint& sp)
463463
return PS_ERROR;
464464
}
465465
}
466-
466+
467+
/* If there are any parameters then compute the number of containers needed
468+
* to execute the command. */
469+
if (params != nullptr)
470+
{
471+
container_count = params->count_containers();
472+
}
473+
467474
return PS_NORMAL;
468475
}
469476

@@ -488,51 +495,36 @@ void MCDispatchCmd::exec_ctxt(MCExecContext &ctxt)
488495
}
489496
else
490497
t_target_ptr = nil;
491-
492-
// Evaluate the parameter list
493-
bool t_success, t_can_debug;
494-
MCParameter *tptr = params;
495-
while (tptr != NULL)
496-
{
497-
// AL-2014-08-20: [[ ArrayElementRefParams ]] Use containers for potential reference parameters
498-
MCAutoPointer<MCContainer> t_container = new (nothrow) MCContainer;
499-
if (tptr -> evalcontainer(ctxt, **t_container))
500-
tptr -> set_argument_container(t_container.Release());
501-
else
502-
{
503-
MCExecValue t_value;
504-
tptr -> clear_argument();
505-
506-
do
507-
{
508-
if (!(t_success = tptr->eval_ctxt(ctxt, t_value)))
509-
t_can_debug = MCB_error(ctxt, line, pos, EE_STATEMENT_BADPARAM);
510-
ctxt.IgnoreLastError();
511-
}
512-
while (!t_success && t_can_debug && (MCtrace || MCnbreakpoints) && !MCtrylock && !MClockerrors);
513-
514-
if (!t_success)
515-
{
516-
ctxt . LegacyThrow(EE_STATEMENT_BADPARAM);
517-
return;
518-
}
519-
520-
tptr->give_exec_argument(t_value);
521-
}
522-
523-
tptr = tptr->getnext();
524-
}
525-
526-
ctxt . SetLineAndPos(line, pos);
527-
MCEngineExecDispatch(ctxt, is_function ? HT_FUNCTION : HT_MESSAGE, *t_message, t_target_ptr, params);
528498

529-
// AL-2014-09-17: [[ Bug 13465 ]] Clear parameters after executing dispatch
530-
tptr = params;
531-
while (tptr != NULL)
532-
{
533-
tptr -> clear_argument();
534-
tptr = tptr->getnext();
499+
/* Attempt to allocate the number of containers needed for the call. */
500+
MCAutoPointer<MCContainer[]> t_containers = new MCContainer[container_count];
501+
if (!t_containers)
502+
{
503+
ctxt.LegacyThrow(EE_NO_MEMORY);
504+
return;
535505
}
506+
507+
/* If the argument list is successfully evaluated, then do the dispatch. */
508+
if (MCKeywordsExecSetupCommandOrFunction(ctxt,
509+
params,
510+
*t_containers,
511+
line,
512+
pos,
513+
is_function))
514+
{
515+
if (!ctxt.HasError())
516+
{
517+
ctxt.SetLineAndPos(line, pos);
518+
MCEngineExecDispatch(ctxt,
519+
is_function ? HT_FUNCTION : HT_MESSAGE,
520+
*t_message,
521+
t_target_ptr,
522+
params);
523+
}
524+
}
525+
526+
/* Clean up the evaluated argument list */
527+
MCKeywordsExecTeardownCommandOrFunction(params);
536528
}
537529

538530
Parse_stat MCMessage::parse(MCScriptPoint &sp)

‎engine/src/exec-engine.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1308,7 +1308,7 @@ static void MCEngineSendOrCall(MCExecContext& ctxt, MCStringRef p_script, MCObje
13081308
MCAutoValueRef t_value;
13091309
MCAutoStringRef t_value_string;
13101310

1311-
if (!t_param_ptr->eval(ctxt, &t_value) ||
1311+
if (!t_param_ptr->eval_argument(ctxt, &t_value) ||
13121312
!ctxt . ConvertToString(*t_value, &t_value_string) ||
13131313
!MCListAppend(*t_param_list, *t_value_string))
13141314
goto cleanup;

‎engine/src/exec-extension.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ Exec_stat MCEngineHandleLibraryMessage(MCNameRef p_message, MCParameter *p_param
448448
if (MCHandlerTypeInfoGetParameterMode(t_signature, i) != kMCHandlerTypeFieldModeOut)
449449
{
450450
MCValueRef t_value;
451-
if (!t_param -> eval(*MCECptr, t_value))
451+
if (!t_param -> eval_argument(*MCECptr, t_value))
452452
{
453453
t_success = false;
454454
break;
@@ -467,6 +467,7 @@ Exec_stat MCEngineHandleLibraryMessage(MCNameRef p_message, MCParameter *p_param
467467

468468
if (!t_arguments . Push(t_value))
469469
{
470+
MCValueRelease(t_value);
470471
t_success = false;
471472
break;
472473
}

‎engine/src/exec-keywords.cpp‎

Lines changed: 67 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -132,43 +132,70 @@ void MCKeywordsExecResolveCommandOrFunction(MCExecContext& ctxt, MCNameRef p_nam
132132
r_handler = t_resolved_handler;
133133
}
134134

135+
bool MCKeywordsExecSetupCommandOrFunction(MCExecContext& ctxt, MCParameter *params, MCContainer *containers, uint2 line, uint2 pos, bool is_function)
136+
{
137+
MCParameter *tptr = params;
138+
uindex_t t_container_index = 0;
139+
while (tptr != NULL)
140+
{
141+
/* If the parameter evaluates as a container, then place the result
142+
* into the next available container slot and bump the index; otherwise
143+
* evaluate as an expression. */
144+
if (tptr -> evalcontainer(ctxt, containers[t_container_index]))
145+
{
146+
tptr -> set_argument_container(&containers[t_container_index]);
147+
t_container_index += 1;
148+
}
149+
else
150+
{
151+
MCExecValue t_value;
152+
153+
if (!ctxt.TryToEvaluateParameter(tptr,
154+
line,
155+
pos,
156+
is_function ? EE_FUNCTION_BADSOURCE : EE_STATEMENT_BADPARAM,
157+
t_value))
158+
{
159+
return false;
160+
}
161+
162+
tptr -> clear_argument();
163+
tptr->give_exec_argument(t_value);
164+
}
165+
166+
tptr = tptr->getnext();
167+
}
168+
169+
return true;
170+
}
171+
172+
void MCKeywordsExecTeardownCommandOrFunction(MCParameter *params)
173+
{
174+
// AL-2014-09-17: [[ Bug 13465 ]] Clear parameters after executing dispatch
175+
MCParameter *tptr = params;
176+
while (tptr != NULL)
177+
{
178+
tptr -> clear_argument();
179+
tptr = tptr->getnext();
180+
}
181+
}
182+
135183
void MCKeywordsExecCommandOrFunction(MCExecContext& ctxt, MCHandler *handler, MCParameter *params, MCNameRef name, uint2 line, uint2 pos, bool global_handler, bool is_function)
136184
{
137185
if (MCscreen->abortkey())
138186
{
139187
ctxt . LegacyThrow(EE_HANDLER_ABORT);
140188
return;
141189
}
142-
190+
143191
if (is_function)
144192
MCexitall = False;
145-
146-
// Go through all the parameters to the function, if they are not variables, clear their current value. Each parameter stores an expression
147-
// which allows its value to be re-evaluated in a given context. Re-evaluate each in the context of ep and set it to the new value.
148-
// As the ep should contain the context of the caller at this point, the expression should be evaluated in that context.
193+
149194
Exec_stat stat;
150-
MCParameter *tptr = params;
151-
while (tptr != NULL)
152-
{
153-
// AL-2014-08-20: [[ ArrayElementRefParams ]] Use containers for potential reference parameters
154-
MCAutoPointer<MCContainer> t_container = new (nothrow) MCContainer;
155-
if (tptr -> evalcontainer(ctxt, **t_container))
156-
tptr -> set_argument_container(t_container.Release());
157-
else
158-
{
159-
tptr -> clear_argument();
160-
MCExecValue t_value;
161-
if (!ctxt . TryToEvaluateParameter(tptr, line, pos, is_function ? EE_FUNCTION_BADSOURCE : EE_STATEMENT_BADPARAM, t_value))
162-
return;
163-
tptr->give_exec_argument(t_value);
164-
}
165-
166-
tptr = tptr->getnext();
167-
}
168-
MCObject *p = ctxt . GetObject();
195+
stat = ES_NOT_HANDLED;
196+
MCObject *p = ctxt . GetObject();
169197
MCExecContext *oldctxt = MCECptr;
170198
MCECptr = &ctxt;
171-
stat = ES_NOT_HANDLED;
172199
Boolean added = False;
173200
if (MCnexecutioncontexts < MAX_CONTEXTS)
174201
{
@@ -218,6 +245,21 @@ void MCKeywordsExecCommandOrFunction(MCExecContext& ctxt, MCHandler *handler, MC
218245
stat = p->handle(HT_FUNCTION, name, params, p);
219246
if (oldstat == ES_PASS && stat == ES_NOT_HANDLED)
220247
stat = ES_PASS;
248+
249+
/* The following clause was pulled in from MCFuncref::eval_ctxt
250+
* it is not quite clear why this code path is different from
251+
* commands; however without the clause a test failure occurs
252+
* in 'TestExtensionLibraryHandlerCallErrors'. */
253+
// MW-2007-08-09: [[ Bug 5705 ]] Throws inside private functions don't trigger an
254+
// exception.
255+
if (!global_handler &&
256+
stat != ES_NORMAL &&
257+
stat != ES_PASS &&
258+
stat != ES_EXIT_HANDLER)
259+
{
260+
MCeerror->add(EE_FUNCTION_BADFUNCTION, line, pos, name);
261+
stat = ES_ERROR;
262+
}
221263
}
222264
else
223265
{
@@ -265,14 +307,6 @@ void MCKeywordsExecCommandOrFunction(MCExecContext& ctxt, MCHandler *handler, MC
265307
ctxt . SetExecStat(stat);
266308
else
267309
ctxt . SetExecStat(ES_NORMAL);
268-
269-
// AL-2014-09-17: [[ Bug 13465 ]] Clear parameters after executing command/function
270-
tptr = params;
271-
while (tptr != NULL)
272-
{
273-
tptr -> clear_argument();
274-
tptr = tptr->getnext();
275-
}
276310
}
277311

278312
////////////////////////////////////////////////////////////////////////////////

‎engine/src/exec.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ along with LiveCode. If not see <http://www.gnu.org/licenses/>. */
3232
#include "variable.h"
3333
#include "handler.h"
3434
#include "hndlrlst.h"
35+
#include "keywords.h"
3536

3637
#include "osspec.h"
3738

‎engine/src/exec.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1802,6 +1802,8 @@ void MCKeywordsExecPass(MCExecContext& ctxt);
18021802
void MCKeywordsExecPassAll(MCExecContext& ctxt);
18031803
void MCKeywordsExecThrow(MCExecContext& ctxt, MCStringRef string);
18041804
void MCKeywordsExecResolveCommandOrFunction(MCExecContext& ctxt, MCNameRef p_name, bool is_function, MCHandler*& r_handler);
1805+
bool MCKeywordsExecSetupCommandOrFunction(MCExecContext& ctxt, MCParameter *params, MCContainer *containers, uint2 line, uint2 pos, bool is_function);
1806+
void MCKeywordsExecTeardownCommandOrFunction(MCParameter *params);
18051807
void MCKeywordsExecCommandOrFunction(MCExecContext& ctxt, MCHandler *handler, MCParameter *params, MCNameRef name, uint2 line, uint2 pos, bool platform_message, bool is_function);
18061808

18071809
////////////////////////////////////////////////////////////////////////////////

‎engine/src/express.cpp‎

Lines changed: 0 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -440,93 +440,3 @@ void MCExpression::initpoint(MCScriptPoint &sp)
440440
}
441441

442442
////////////////////////////////////////////////////////////////////////////////
443-
444-
MCFuncref::MCFuncref(MCNameRef inname)
445-
: name(inname)
446-
{
447-
handler = nil;
448-
params = NULL;
449-
resolved = false;
450-
global_handler = false;
451-
}
452-
453-
MCFuncref::~MCFuncref()
454-
{
455-
while (params != NULL)
456-
{
457-
MCParameter *tmp = params;
458-
params = params->getnext();
459-
delete tmp;
460-
}
461-
}
462-
463-
Parse_stat MCFuncref::parse(MCScriptPoint &sp, Boolean the)
464-
{
465-
initpoint(sp);
466-
if (getparams(sp, &params) != PS_NORMAL)
467-
{
468-
MCperror->add(PE_FUNCTION_BADPARAMS, sp);
469-
return PS_ERROR;
470-
}
471-
472-
if (MCIsGlobalHandler(*name))
473-
{
474-
global_handler = true;
475-
resolved = true;
476-
}
477-
478-
return PS_NORMAL;
479-
}
480-
481-
void MCFuncref::eval_ctxt(MCExecContext& ctxt, MCExecValue& r_value)
482-
{
483-
if (!resolved)
484-
{
485-
MCKeywordsExecResolveCommandOrFunction(ctxt, *name, true, handler);
486-
resolved = true;
487-
}
488-
489-
MCKeywordsExecCommandOrFunction(ctxt, handler, params, *name, line, pos, global_handler, true);
490-
491-
Exec_stat stat = ctxt . GetExecStat();
492-
493-
// MW-2007-08-09: [[ Bug 5705 ]] Throws inside private functions don't trigger an
494-
// exception.
495-
if (stat != ES_NORMAL && stat != ES_PASS && stat != ES_EXIT_HANDLER)
496-
{
497-
ctxt . LegacyThrow(EE_FUNCTION_BADFUNCTION, *name);
498-
return;
499-
}
500-
501-
if (MCresultmode == kMCExecResultModeReturn)
502-
{
503-
if (MCresult->eval(ctxt, r_value . valueref_value))
504-
{
505-
r_value . type = kMCExecValueTypeValueRef;
506-
return;
507-
}
508-
}
509-
else if (MCresultmode == kMCExecResultModeReturnValue)
510-
{
511-
// Our return value is MCresult, and 'the result' gets set to empty.
512-
if (MCresult->eval(ctxt, r_value . valueref_value))
513-
{
514-
r_value . type = kMCExecValueTypeValueRef;
515-
ctxt.SetTheResultToEmpty();
516-
return;
517-
}
518-
}
519-
else if (MCresultmode == kMCExecResultModeReturnError)
520-
{
521-
// Our return value is empty, and 'the result' remains as it is.
522-
MCExecTypeSetValueRef(r_value, MCValueRetain(kMCEmptyString));
523-
524-
// Make sure we reset the 'return mode' to default.
525-
MCresultmode = kMCExecResultModeReturn;
526-
return;
527-
}
528-
529-
ctxt . Throw();
530-
}
531-
532-
////////////////////////////////////////////////////////////////////////////////

0 commit comments

Comments
 (0)