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

Commit 8677165

Browse files
committed
[[ Bug 19307 ]] Invalidate children of deleted object
When an object is deleted, all its children should be inaccessible from the point of view of the object tree, so recursively invalidate all the weak proxies as necessary. Also ensure that players are closed and stopped when a parent is deleted.
1 parent 51ca551 commit 8677165

9 files changed

Lines changed: 68 additions & 71 deletions

File tree

docs/notes/bugfix-19307.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Prevent crash when saving standalone while player is playing

engine/src/control.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,18 @@ Boolean MCControl::del(bool p_check_flag)
497497
if (getselected())
498498
getcard()->dirtyselection(rect);
499499

500+
// IM-2012-05-16 [[ BZ 10212 ]] deleting the dragtarget control in response
501+
// to a 'dragdrop' message would leave these globals pointing to the deleted
502+
// object, leading to an infinite loop if the target was a field
503+
if (MCdragdest == this)
504+
{
505+
MCdragdest = nil;
506+
MCdropfield = nil;
507+
}
508+
509+
if (MCdragsource == this)
510+
MCdragsource = nil;
511+
500512
switch (parent->gettype())
501513
{
502514
case CT_STACK:
@@ -519,18 +531,6 @@ Boolean MCControl::del(bool p_check_flag)
519531
default:
520532
MCUnreachable();
521533
}
522-
523-
// IM-2012-05-16 [[ BZ 10212 ]] deleting the dragtarget control in response
524-
// to a 'dragdrop' message would leave these globals pointing to the deleted
525-
// object, leading to an infinite loop if the target was a field
526-
if (MCdragdest == this)
527-
{
528-
MCdragdest = nil;
529-
MCdropfield = nil;
530-
}
531-
532-
if (MCdragsource == this)
533-
MCdragsource = nil;
534534

535535
// MCObject now does things on del(), so we must make sure we finish by
536536
// calling its implementation.

engine/src/object.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,19 @@ void MCObject::removereferences()
861861
MCscreen->cancelmessageobject(this, NULL);
862862
removefrom(MCfrontscripts);
863863
removefrom(MCbackscripts);
864+
865+
// If the object is marked as being used as a parentScript, flush the parentScript
866+
// table so we don't get any dangling pointers.
867+
if (m_is_parent_script)
868+
{
869+
MCParentScript::FlushObject(this);
870+
m_is_parent_script = false;
871+
}
872+
873+
// This object is in the process of being deleted; invalidate any weak refs
874+
// and prevent any new ones from being created.
875+
m_weak_proxy->Clear();
876+
m_weak_proxy = nil;
864877
}
865878

866879
bool MCObject::isdeletable(bool p_check_flag)
@@ -877,19 +890,6 @@ bool MCObject::isdeletable(bool p_check_flag)
877890

878891
Boolean MCObject::del(bool p_check_flag)
879892
{
880-
// If the object is marked as being used as a parentScript, flush the parentScript
881-
// table so we don't get any dangling pointers.
882-
if (m_is_parent_script)
883-
{
884-
MCParentScript::FlushObject(this);
885-
m_is_parent_script = false;
886-
}
887-
888-
// This object is in the process of being deleted; invalidate any weak refs
889-
// and prevent any new ones from being created.
890-
m_weak_proxy->Clear();
891-
m_weak_proxy = nil;
892-
893893
return True;
894894
}
895895

engine/src/objptr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ void MCObjptr::setparent(MCObject *newparent)
8080
MCObjectHandle MCObjptr::Get()
8181
{
8282
// If the object pointer hasn't yet been bound, resolve it using the ID
83-
if (!m_objptr.IsBound())
83+
if (!m_objptr.IsBound() && m_parent . IsValid())
8484
{
8585
// Note that this may return null if the control doesn't exist
8686
m_objptr = m_parent->getstack()->getcontrolid(CT_LAYER, m_id);

engine/src/player-legacy.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ class MCPlayer : public MCControl, public MCMixinObjectHandle<MCPlayer>, public
130130
// MW-2011-09-06: [[ Redraw ]] Added 'sprite' option - if true, ink and opacity are not set.
131131
virtual void draw(MCDC *dc, const MCRectangle &dirty, bool p_isolated, bool p_sprite);
132132

133+
virtual void removereferences(void);
134+
void removefromplayers(void);
135+
133136
// virtual functions from MCControl
134137
IO_stat load(IO_handle stream, uint32_t version);
135138
IO_stat extendedload(MCObjectInputStream& p_stream, uint32_t version, uint4 p_length);

engine/src/player-platform.cpp

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -875,29 +875,7 @@ MCPlayer::MCPlayer(const MCPlayer &sref) : MCControl(sref)
875875
}
876876

877877
MCPlayer::~MCPlayer()
878-
{
879-
// OK-2009-04-30: [[Bug 7517]] - Ensure the player is actually closed before deletion, otherwise dangling references may still exist.
880-
while (opened)
881-
close();
882-
883-
playstop();
884-
885-
// MW-2014-07-16: [[ Bug ]] Remove the player from the player's list.
886-
if (MCplayers)
887-
{
888-
if (MCplayers == this)
889-
MCplayers = nextplayer;
890-
else
891-
{
892-
MCPlayer *tptr = MCplayers;
893-
while (tptr->nextplayer && tptr->nextplayer != this)
894-
tptr = tptr->nextplayer;
895-
if (tptr->nextplayer == this)
896-
tptr->nextplayer = nextplayer;
897-
}
898-
}
899-
nextplayer = nil;
900-
878+
{
901879
if (m_platform_player != nil)
902880
MCPlatformPlayerRelease(m_platform_player);
903881

@@ -1737,22 +1715,6 @@ Boolean MCPlayer::playstop()
17371715

17381716
freetmp();
17391717

1740-
/*
1741-
if (MCplayers != NULL)
1742-
{
1743-
if (MCplayers == this)
1744-
MCplayers = nextplayer;
1745-
else
1746-
{
1747-
MCPlayer *tptr = MCplayers;
1748-
while (tptr->nextplayer != NULL && tptr->nextplayer != this)
1749-
tptr = tptr->nextplayer;
1750-
if (tptr->nextplayer == this)
1751-
tptr->nextplayer = nextplayer;
1752-
}
1753-
}
1754-
nextplayer = NULL;*/
1755-
17561718
if (disposable)
17571719
{
17581720
//if (needmessage)

engine/src/player-platform.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ class MCPlayer : public MCControl, public MCMixinObjectHandle<MCPlayer>, public
133133
// MW-2011-09-06: [[ Redraw ]] Added 'sprite' option - if true, ink and opacity are not set.
134134
virtual void draw(MCDC *dc, const MCRectangle &dirty, bool p_isolated, bool p_sprite);
135135

136+
virtual void removereferences(void);
137+
void removefromplayers(void);
138+
136139
// virtual functions from MCControl
137140
IO_stat load(IO_handle stream, uint32_t version);
138141
IO_stat extendedload(MCObjectInputStream& p_stream, uint32_t version, uint4 p_length);

engine/src/player-srv-stubs.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,6 @@ MCPlayer::MCPlayer(const MCPlayer &sref) : MCControl(sref)
8686

8787
MCPlayer::~MCPlayer()
8888
{
89-
// OK-2009-04-30: [[Bug 7517]] - Ensure the player is actually closed before deletion, otherwise dangling references may still exist.
90-
while (opened)
91-
close();
92-
93-
playstop();
94-
9589
MCValueRelease(filename);
9690
MCValueRelease(userCallbackStr);
9791
}

engine/src/player.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
#include "player.h"
2525
#include "exec.h"
2626

27+
#include "globals.h"
28+
2729
////////////////////////////////////////////////////////////////////////////////
2830

2931
MCPropertyInfo MCPlayer::kProperties[] =
@@ -78,3 +80,35 @@ MCObjectPropertyTable MCPlayer::kPropertyTable =
7880
&kProperties[0],
7981
};
8082

83+
void MCPlayer::removereferences()
84+
{
85+
// OK-2009-04-30: [[Bug 7517]] - Ensure the player is actually closed before deletion, otherwise dangling references may still exist.
86+
while (opened)
87+
close();
88+
89+
playstop();
90+
91+
removefromplayers();
92+
93+
MCObject::removereferences();
94+
}
95+
96+
void MCPlayer::removefromplayers()
97+
{
98+
if (MCplayers)
99+
{
100+
if (MCplayers == this)
101+
MCplayers = nextplayer;
102+
else
103+
{
104+
MCPlayer *tptr = MCplayers;
105+
while (tptr->nextplayer.IsBound() && tptr->nextplayer != this)
106+
tptr = tptr->nextplayer;
107+
108+
if (tptr->nextplayer == this)
109+
tptr->nextplayer = nextplayer;
110+
}
111+
}
112+
nextplayer = nullptr;
113+
}
114+

0 commit comments

Comments
 (0)