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

Leak with using weak NSThread in thread #5

Open
newacct opened this issue Feb 14, 2013 · 4 comments
Open

Leak with using weak NSThread in thread #5

newacct opened this issue Feb 14, 2013 · 4 comments

Comments

@newacct
Copy link

newacct commented Feb 14, 2013

On profiling an app with the following code, running on iOS 4.3 simulator with PLWeakCompatibility, I am getting a leak in the Leaks instrument, saying that the struct TLS (as well as the dictionaries inside it) created in GetTLS are leaked.

Is this a bug with PLWeakCompatibility, or am I doing something wrong?

- (void)foo {
  @autoreleasepool {
    NSThread* __weak myThread = [NSThread currentThread];
    [myThread self];
  }
}

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
  [NSThread detachNewThreadSelector:@selector(foo) toTarget:self withObject:nil];
  return YES;
}
@mikeash
Copy link
Member

mikeash commented Feb 20, 2013

These shouldn't be leaked, but it's always possible there's a mistake. For reference, here's the cleanup code:

https://github.com/plausiblelabs/PLWeakCompatibility/blob/master/PLWeakCompatibility/PLWeakCompatibilityStubs.m#L328

If you put the above code in a loop and spin off e.g. 1000 threads, do you then get 1000 leaks?

@newacct
Copy link
Author

newacct commented Feb 25, 2013

Yes, if you do this n times in a loop, then you get approximately n leaks.

I've looked into what happens. Basically (ignoring what happens on the main thread), on the new thread we created, we create the struct TLS, destroy it, and then create it again, and never destroy it again. Here is a stack trace of each event:

struct TLS created:

GetTLS
SwizzledReleaseIMP
__arclite_objc_release
-[MyAppDelegate foo]
-[NSThread main]
__NSThread__main__
_pthread_start

This is when the thread is assigned to the weak pointer. This is expected.

struct TLS destroyed:

DestroyTLS
_pthread_tsd_cleanup
_pthread_exit
pthread_exit
+[NSThread exit]
__NSThread__main__
_pthread_start

This is when the thread exits, and the struct TLS is cleaned up. This is also expected.

struct TLS created again:

GetTLS
SwizzledReleaseIMP
-[NSConcreteNotification dealloc]
__NSFinalizeThreadData
_pthread_tsd_cleanup
_pthread_exit
pthread_exit
+[NSThread exit]
__NSThread__main__
_pthread_start

This is the leak. What appears to be happening is that PLWeakCompatibility is not the only one using thread-local storage. Cocoa apparently uses thread-local storage for some NSThread data, including an NSConcreteNotification. This particular thing's cleanup happens to run after PLWeakCompatibility's cleanup. However, this thing's cleanup happens to also do a -release on our thread object, which (since we used weak on threads) goes into SwizzledReleaseIMP, which tries to get the TLS (which, since we've already cleaned it up, re-creates it). The problem is that this is occurring after PLWeakCompatibility's cleanup, so it won't get cleaned up again. So it is leaked.

@mikeash
Copy link
Member

mikeash commented Mar 2, 2013

Great detective work. I'll have to think about how this could potentially be fixed. In the meantime, is "don't use __weak on NSThread" a sufficient workaround? Definitely want to fix this, but it seems that NSThread should be a fairly rare target for weak references.

@mahaiyannn
Copy link

If my app use this library, will the apple reject my app?

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

No branches or pull requests

3 participants