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

Commit 14575d2

Browse files
runrevmarkrunrevmark
committed
[[ Bug 19587 ]] Relax object execution lock
This patch relaxes the object execution lock as far as it can. Locks are only taken on an object if it is executing a handler, or if one of its behavior's is executing a handler. In all other cases (e.g. handling a message in an owning object after the message passes), deleting the target will work, but the target will be unset afterwards. If the target is deleted at any point in the message path, the current message will not pass - regardless of whether pass was invoked or not. This is safe as any attempt to use 'the target' after it has been deleted will throw an error as there is no object to resolve anymore.
1 parent eeef869 commit 14575d2

File tree

7 files changed

+67
-34
lines changed

7 files changed

+67
-34
lines changed

‎docs/notes/bugfix-19587.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# Fix deletion of the target in safe cases
2+
3+
You can now safely 'delete the target' as long as there are
4+
no handlers on the stack owned by the target.
5+
6+
After deleting 'the target', 'the target' will become
7+
empty which will result in an execution error when an attempt
8+
is made to dereference it.

‎engine/src/dispatch.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,15 @@ Exec_stat MCDispatch::handle(Handler_type htype, MCNameRef mess, MCParameter *pa
256256

257257
if (oldstat == ES_PASS && stat == ES_NOT_HANDLED)
258258
stat = ES_PASS;
259+
260+
if (stat == ES_PASS || stat == ES_NOT_HANDLED)
261+
{
262+
if (!MCtargetptr.IsValid())
263+
{
264+
stat = ES_NORMAL;
265+
t_has_passed = false;
266+
}
267+
}
259268
}
260269

261270
//#ifdef TARGET_SUBPLATFORM_IPHONE
@@ -285,6 +294,15 @@ Exec_stat MCDispatch::handle(Handler_type htype, MCNameRef mess, MCParameter *pa
285294
{
286295
extern Exec_stat MCEngineHandleLibraryMessage(MCNameRef name, MCParameter *params);
287296
stat = MCEngineHandleLibraryMessage(mess, params);
297+
298+
if (stat == ES_PASS || stat == ES_NOT_HANDLED)
299+
{
300+
if (!MCtargetptr.IsValid())
301+
{
302+
stat = ES_NORMAL;
303+
t_has_passed = false;
304+
}
305+
}
288306
}
289307

290308
if (MCmessagemessages && stat != ES_PASS && MCtargetptr)

‎engine/src/exec-engine.cpp‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,24 +1226,13 @@ void MCEngineExecDispatch(MCExecContext& ctxt, int p_handler_type, MCNameRef p_m
12261226
MCdynamicpath = MCdynamiccard.IsValid();
12271227
if (t_stat == ES_PASS || t_stat == ES_NOT_HANDLED)
12281228
{
1229-
/* If the target object was deleted in the frontscript, prevent
1230-
* normal message dispatch as if the frontscript did not pass the
1231-
* message. */
1232-
if (t_object)
1229+
switch(t_stat = t_object -> handle((Handler_type)p_handler_type, p_message, p_parameters, t_object.Get()))
12331230
{
1234-
MCObjectExecutionLock t_object_lock(t_object);
1235-
switch(t_stat = t_object -> handle((Handler_type)p_handler_type, p_message, p_parameters, t_object.Get()))
1236-
{
1237-
case ES_ERROR:
1238-
ctxt . LegacyThrow(EE_DISPATCH_BADCOMMAND, p_message);
1239-
break;
1240-
default:
1241-
break;
1242-
}
1243-
}
1244-
else
1245-
{
1246-
t_stat = ES_NORMAL;
1231+
case ES_ERROR:
1232+
ctxt . LegacyThrow(EE_DISPATCH_BADCOMMAND, p_message);
1233+
break;
1234+
default:
1235+
break;
12471236
}
12481237
}
12491238

‎engine/src/object.cpp‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,14 @@ Exec_stat MCObject::handleself(Handler_type p_handler_type, MCNameRef p_message,
11371137
if (t_stat == ES_ERROR || t_stat == ES_EXIT_ALL)
11381138
return t_stat;
11391139
}
1140+
1141+
if (t_stat == ES_PASS || t_stat == ES_NOT_HANDLED)
1142+
{
1143+
if (!MCtargetptr.IsValid())
1144+
{
1145+
t_main_stat = ES_NORMAL;
1146+
}
1147+
}
11401148

11411149
// Return the result of executing the main handler in the object
11421150
return t_main_stat;
@@ -2081,7 +2089,8 @@ Exec_stat MCObject::dispatch(Handler_type p_type, MCNameRef p_message, MCParamet
20812089
MCAssert(!MCtargetptr || MCtargetptr.IsBoundTo(this));
20822090
if (MCtargetptr)
20832091
{
2084-
MCObjectExecutionLock t_target_lock(this);
2092+
// MAY-DELETE: Handle the message - this (MCtargetptr) might be unbound
2093+
// after this call if it is deleted.
20852094
t_stat = handle(p_type, p_message, p_params, this);
20862095
}
20872096
else
@@ -2163,8 +2172,8 @@ Exec_stat MCObject::message(MCNameRef mess, MCParameter *paramptr, Boolean chang
21632172
* frontscript did not pass the message. */
21642173
if (t_self_handle)
21652174
{
2166-
MCObjectExecutionLock t_self_lock(this);
2167-
// PASS STATE FIX
2175+
// MAY-DELETE: Handle the message - this might be unbound after
2176+
// this call if it is deleted.
21682177
Exec_stat oldstat = stat;
21692178
stat = handle(HT_MESSAGE, mess, paramptr, this);
21702179
if (oldstat == ES_PASS && stat == ES_NOT_HANDLED)

‎engine/src/objectprops.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,8 @@ Exec_stat MCObject::sendgetprop(MCExecContext& ctxt, MCNameRef p_set_name, MCNam
351351
MCAssert(!MCtargetptr || MCtargetptr.IsBoundTo(this));
352352
if (MCtargetptr)
353353
{
354-
MCObjectExecutionLock t_self_lock(this);
354+
// MAY-DELETE: Handle the message - this/MCtargetptr might be unbound after
355+
// this call if it is deleted.
355356
t_stat = handle(HT_GETPROP, t_getprop_name, &p1, this);
356357
}
357358
else
@@ -455,7 +456,8 @@ Exec_stat MCObject::sendsetprop(MCExecContext& ctxt, MCNameRef p_set_name, MCNam
455456
MCAssert(!MCtargetptr || MCtargetptr.IsBoundTo(this));
456457
if (MCtargetptr)
457458
{
458-
MCObjectExecutionLock t_self_lock(this);
459+
// MAY-DELETE: Handle the message - this/MCtargetptr might be unbound after
460+
// this call if it is deleted.
459461
t_stat = handle(HT_SETPROP, t_setprop_name, &p1, this);
460462
}
461463
else

‎engine/src/stack.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,6 +1596,13 @@ Exec_stat MCStack::handle(Handler_type htype, MCNameRef message, MCParameter *pa
15961596
stat = m_externals -> Handle(this, htype, message, params);
15971597
if (oldstat == ES_PASS && stat == ES_NOT_HANDLED)
15981598
stat = ES_PASS;
1599+
if (stat == ES_PASS || stat == ES_NOT_HANDLED)
1600+
{
1601+
if (!MCtargetptr.IsValid())
1602+
{
1603+
stat = ES_NORMAL;
1604+
}
1605+
}
15991606
}
16001607

16011608
// MW-2011-06-30: Cleanup of message path - this clause handles the transition

‎tests/lcs/core/engine/target.livecodescript‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,23 @@ along with LiveCode. If not see <http://www.gnu.org/licenses/>. */
1919
on TestSetup
2020
create button "script"
2121
set the script of button "script" to "on doSomething; delete the target; pass doSomething; end doSomething"
22-
22+
2323
create button "test"
2424
end TestSetup
2525

2626
on TestDeleteTargetInFrontScript
2727
insert the script of button "script" into front
28-
set the script of button "test" to "on doSomething; send __TestDeletedCallback; end doSomething"
29-
__TestDeleteTarget
28+
dispatch "doSomething" to button "test"
3029
TestAssert "Delete target in frontscript", there is no button "test"
31-
TestAssert "Delete target in frontscript blocks message", sDeleted is not true
30+
TestAssert "Delete target in frontscript blocks message", it is "handled"
3231
remove the script of button "script" from front
3332
end TestDeleteTargetInFrontScript
3433

3534
on TestDeleteTargetInBackScript
3635
insert the script of button "script" into back
37-
TestAssertThrow "Delete target in backscript", "__TestDeleteTarget", the long id me, 347
36+
dispatch "doSomething" to button "test"
37+
TestAssert "Delete target in backscript", there is no button "test"
38+
TestAssert "Delete target in backscript blocks message", it is "handled"
3839
remove the script of button "script" from back
3940
end TestDeleteTargetInBackScript
4041

@@ -45,15 +46,14 @@ on TestDeleteTargetInBehavior
4546
end TestDeleteTargetInBehavior
4647

4748
on TestDeleteTargetInOwner
48-
set the script of the owner of btn "test" to "on doSomething; delete the target; end doSomething"
49-
TestAssertThrow "Delete target in owner", "__TestDeleteTarget", the long id me, 347
49+
set the script of the owner of btn "test" to "on doSomething; delete the target; pass doSomething; end doSomething"
50+
dispatch "doSomething" to button "test"
51+
TestAssert "Delete target in owner", there is no button "test"
52+
TestAssert "Delete target in owner blocks message", it is "handled"
5053
end TestDeleteTargetInOwner
5154

55+
/**/
56+
5257
on __TestDeleteTarget
5358
send "doSomething" to button "test"
5459
end __TestDeleteTarget
55-
56-
local sDeleted
57-
on __TestDeletedCallback
58-
put true into sDeleted
59-
end __TestDeletedCallback

0 commit comments

Comments
 (0)