Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue when object dealloc in DFS #59

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zhongwuzw
Copy link
Contributor

We use the NSMutableSet to save objects when we DFS, NSMutableSet use hash to distinguish different objects, in our situation, we use FBNodeEnumerator->FBObjectiveCGraphElement->object's pointer address, but object can be dealloc anytime, refer to Apple Document and tests, we can not add mutable object to a collection.

After my tests, for example:
objectsOnPath has a FBNodeEnumerator object A, after some step when DFS, we caught A again, after that, A.object.object dealloc, A's hash is changed to 0. but objectsOnPath haven't realize that change, it leads to containsObject not accurate when we call [objectsOnPath containsObject: A] .

@@ -112,31 +112,33 @@ - (void)addCandidate:(id)candidate
@autoreleasepool {
// Take topmost node in stack and mark it as visited
FBNodeEnumerator *top = [stack lastObject];
// Take object that topmost node monitor
__weak id object = top.object.object;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it needs to mimic storeWeak?


// Take next adjecent node to that child. Wrapper object can
// persist iteration state. If we see that node again, it will
// give us new adjacent node unless it runs out of them
FBNodeEnumerator *firstAdjacent = [top nextObject];
if (firstAdjacent) {
__weak id firstAdjacentObject = firstAdjacent.object.object;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it needs to mimic storeWeak?

@suxinde2009
Copy link

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants