Author: thebeing
Date: Fri Jan 29 14:42:07 2016
New Revision: 39318
URL: http://svn.gna.org/viewcvs/gnustep?rev=39318&view=rev
Log:
Add automatic unregistration of threads that have not been
been explicitly unregistered. This works by keeping around
a map table with all threads currently undergoing cleanup,
and using that as a fallback if pthread_getspecific would
not return the NSThread object from TLS.
Added:
libs/base/trunk/Tests/base/NSThread/late_unregister.m
- copied, changed from r39317,
libs/base/trunk/Tests/base/NSThread/lazy_thread.m
Modified:
libs/base/trunk/ChangeLog
libs/base/trunk/Source/NSThread.m
Modified: libs/base/trunk/ChangeLog
URL:
http://svn.gna.org/viewcvs/gnustep/libs/base/trunk/ChangeLog?rev=39318&r1=39317&r2=39318&view=diff
==============================================================================
--- libs/base/trunk/ChangeLog (original)
+++ libs/base/trunk/ChangeLog Fri Jan 29 14:42:07 2016
@@ -1,3 +1,12 @@
+2016-01-29 Niels Grewe <[email protected]>
+
+ * Source/NSThread.m: Automatic 'late' unregistration of thread objects:
When
+ +exit is not called, we no longer log a warning and leak the thread
object.
+ Instead, we add it to a map table, keyed under the current thread ID,
and
+ use that in GSCurrentThread() to find the correct NSThread object if
+ pthread_getspecific wont return it to us.
+ * Tests/base/NSThread/late_unregister.m: Test case for late
unregistration.
+
2016-01-21 Niels Grewe <[email protected]>
* Source/Additions/GSMime.m: Fix folding of headers containing
@@ -394,7 +403,7 @@
* Source/Additions/Unicode.m
* Tools/AGSOutput.m
return NULL or nil instead of NO where pointers are to
- be returned
+ be returned
2015-05-26 Richard Frith-Macdonald <[email protected]>
@@ -513,7 +522,7 @@
2015-03-10 Niels Grewe <[email protected]>
- * Source/GSTimSort.m: Fix a DoS vulnerability discovered in the
+ * Source/GSTimSort.m: Fix a DoS vulnerability discovered in the
Timsort algorithm. For information about the problem please refer to
http://www.envisage-project.eu/proving-android-java-and-python-sorting
-algorithm-is-broken-and-how-to-fix-it/
@@ -576,7 +585,7 @@
2015-01-13 Marcus Mueller <[email protected]>
- * Headers/Foundation/Foundation.h: added NSUUID.h
+ * Headers/Foundation/Foundation.h: added NSUUID.h
2014-12-28 Wolfgang Lux <[email protected]>
@@ -756,7 +765,7 @@
* Tools/make_strings/GNUmakefile (CONFIG_SYSTEM_LIBS): Define
to the empty string to avoid linking the tools against
external libraries.
-
+
2014-06-30 Yavor Doganov <[email protected]>
* base.make.in: make base dependencies explicit only when statically
Modified: libs/base/trunk/Source/NSThread.m
URL:
http://svn.gna.org/viewcvs/gnustep/libs/base/trunk/Source/NSThread.m?rev=39318&r1=39317&r2=39318&view=diff
==============================================================================
--- libs/base/trunk/Source/NSThread.m (original)
+++ libs/base/trunk/Source/NSThread.m Fri Jan 29 14:42:07 2016
@@ -74,6 +74,7 @@
#import "Foundation/NSInvocation.h"
#import "Foundation/NSUserDefaults.h"
#import "Foundation/NSGarbageCollector.h"
+#import "Foundation/NSValue.h"
#import "GSPrivate.h"
#import "GSRunLoopCtxt.h"
@@ -126,7 +127,7 @@
}
#if 0
-/*
+/*
* NSThread setName: method for windows.
* FIXME ... This is code for the microsoft compiler;
* how do we make it work for gcc/clang?
@@ -157,7 +158,7 @@
info.dwFlags = 0;
__try
- {
+ {
RaiseException(MS_VC_EXCEPTION, 0,
sizeof(info)/sizeof(ULONG_PTR), (ULONG_PTR*)&info);
result = 0;
@@ -391,41 +392,210 @@
static pthread_key_t thread_object_key;
+
+/**
+ * pthread_t is an opaque type. It might be a scalar type or
+ * some kind of struct depending on the implementation, so we
+ * need to wrap it up in an NSValue object if we want to pass
+ * it around.
+ * This follows the CoreFoundation 'create rule' and returns an object with
+ * a reference count of 1.
+ */
+static inline NSValue* NSValueCreateFromPthread(pthread_t thread)
+{
+ return [[NSValue alloc] initWithBytes: &thread
+ objCType: @encode(pthread_t)];
+}
+
+/**
+ * Conversely, we need to be able to retrieve the pthread_t
+ * from an NSValue.
+ */
+static inline void _getPthreadFromNSValue(NSValue* value, pthread_t
*thread_ptr)
+{
+ NSCAssert(thread_ptr, @"No storage for thread reference");
+# ifndef NS_BLOCK_ASSERTIONS
+ const char* enc = [value objCType];
+ NSCAssert(enc != NULL && (0 == strcmp(@encode(pthread_t),enc)),
+ @"Invalid NSValue container for thread reference");
+# endif
+ [value getValue: (void*)thread_ptr];
+}
+
+/**
+ * This is the comparison function for boxed pthreads, as used by the
+ * NSMapTable containing them.
+ */
+static BOOL
+_boxedPthreadIsEqual(NSMapTable *t,
+ const void* boxed,
+ const void* boxedOther)
+{
+ pthread_t thread;
+ pthread_t otherThread;
+ _getPthreadFromNSValue(boxed, &thread);
+ _getPthreadFromNSValue(boxedOther, &otherThread);
+ return pthread_equal(thread, otherThread);
+}
+
+/**
+ * Since pthread_t is opaque, we cannot make any assumption about how
+ * to hash it. There are a few problems here:
+ * 1. Functions to obtain the thread ID of an arbitrary thread
+ * exist in the in the Win32 and some pthread APIs (GetThreadId() and
+ * pthread_getunique_np(), respectively), but there is no protable solution
+ * for this problem.
+ * 2. Even where pthread_getunique_np() is available, it might have different
+ * definitions, so it's not really robust to use it.
+ *
+ * For these reasons, we always return the same hash. That fulfills the API
+ * contract for NSMapTable (key-hash equality as a necessary condition for key
+ * equality), but makes things quite inefficient (linear search over all
+ * elements), so we need to keep the table small.
+ */
+static NSUInteger _boxedPthreadHash(NSMapTable* t, const void* value)
+{
+ return 0;
+}
+
+/**
+ * Retain callback for boxed thread references.
+ */
+static void _boxedPthreadRetain(NSMapTable* t, const void* value)
+{
+ [(NSValue*)value retain];
+}
+
+/**
+ * Release callback for boxed thread references.
+ */
+static void _boxedPthreadRelease(NSMapTable* t, void* value)
+{
+ [(NSValue*)value release];
+}
+
+/**
+ * Description callback for boxed thread references.
+ */
+static NSString *_boxedPthreadDescribe(NSMapTable* t, const void* value)
+{
+ return [(NSValue*)value description];
+}
+
+
+static const NSMapTableKeyCallBacks _boxedPthreadKeyCallBacks =
+{
+ _boxedPthreadHash,
+ _boxedPthreadIsEqual,
+ _boxedPthreadRetain,
+ _boxedPthreadRelease,
+ _boxedPthreadDescribe,
+ NULL
+};
+
+
+/**
+ * This map table maintains a list of all threads currently undergoing
+ * cleanup. This is a required so that +currentThread can still find the
+ * thred if called from within the late-cleanup function.
+ */
+static NSMapTable *_exitingThreads;
+static NSLock *_exitingThreadsLock;
+
+/**
+ * Called before late cleanup is run and inserts the NSThread object into the
+ * table that is used by GSCurrentThread to find the thread if it is called
+ * during cleanup. The boxedThread variable contains a boxed reference to
+ * the result of calling pthread_self().
+ */
+static inline void _willLateUnregisterThread(NSValue *boxedThread,
+ NSThread *specific)
+{
+ [_exitingThreadsLock lock];
+ NS_DURING
+ {
+ NSMapInsert(_exitingThreads, (const void*)boxedThread,
+ (const void*)specific);
+ }
+ NS_HANDLER
+ {
+ [_exitingThreadsLock unlock];
+ [localException raise];
+ }
+ NS_ENDHANDLER
+ [_exitingThreadsLock unlock];
+}
+
+/**
+ * Called after late cleanup has run. Will remove the current thread from
+ * the lookup table again. The boxedThread variable contains a boxed reference
+ * to the result of calling pthread_self().
+ */
+static inline void _didLateUnregisterCurrentThread(NSValue *boxedThread)
+{
+ [_exitingThreadsLock lock];
+ NS_DURING
+ {
+ NSMapRemove(_exitingThreads, (const void*)boxedThread);
+ }
+ NS_HANDLER
+ {
+ [_exitingThreadsLock unlock];
+ [localException raise];
+ }
+ NS_ENDHANDLER
+ [_exitingThreadsLock unlock];
+}
+
+/*
+ * Forward declaration of the thread unregistration function
+ */
+static void
+unregisterActiveThread(NSThread *thread);
+
/**
* Pthread cleanup call.
*
* We should normally not get here ... because threads should exit properly
* and clean up, so that this function doesn't get called. However if a
* thread terminates for some reason without calling the exit method, we
- * can at least log it.
- *
- * We can't do anything more than that since at the point
- * when this function is called, the thread specific data is no longer
- * available, so the currentThread method will always fail and the
- * repercussions of that would well be a crash.
- *
- * As a special case, we ignore the exit of the default thread ... that one
- * will usually terminate without calling the exit method as it ends the
- * whole process by returning from the 'main' function.
+ * we add it to a special lookup table that is used by GSCurrentThread() to
+ * obtain the NSThread object.
+ * We need to be a bit careful about this regarding object allocation because
+ * we must not call into NSAutoreleasePool unless the NSThread object can still
+ * be found using GSCurrentThread()
*/
static void exitedThread(void *thread)
{
+
if (thread != defaultThread)
{
- NSUInteger tid;
-
-#if defined(__MINGW__)
- tid = (NSUInteger)GetCurrentThreadId();
-#elif defined(HAVE_GETTID)
- tid = (NSUInteger)syscall(SYS_gettid);
-#elif defined(HAVE_PTHREAD_GETTHREADID_NP)
- tid = (NSUInteger)pthread_getthreadid_np();
-#else
- tid = (NSUInteger)thread;
-#endif
-
- fprintf(stderr, "WARNING thread %"PRIuPTR
- " terminated without calling +exit!\n", tid);
+ [(NSThread*)thread retain];
+ NSValue *ref = NSValueCreateFromPthread(pthread_self());
+ _willLateUnregisterThread(ref, (NSThread*)thread);
+ /* We create a pool for all objects used during cleanup to go into.
+ */
+ NSAutoreleasePool *arp = [NSAutoreleasePool new];
+ NS_DURING
+ {
+ unregisterActiveThread((NSThread*)thread);
+ }
+ NS_HANDLER
+ {
+ DESTROY(arp);
+ _didLateUnregisterCurrentThread(ref);
+ DESTROY(ref);
+ [(NSThread*)thread release];
+ }
+ NS_ENDHANDLER
+ DESTROY(arp);
+
+ /* At this point threre shouldn't be any autoreleased objects lingering
+ * around anymore. So we may remove the thread from the lookup table.
+ */
+ _didLateUnregisterCurrentThread(ref);
+ DESTROY(ref);
+ [(NSThread*)thread release];
}
}
@@ -437,6 +607,24 @@
GSCurrentThread(void)
{
NSThread *thr = pthread_getspecific(thread_object_key);
+ if (nil == thr)
+ {
+ NSValue *selfThread = NSValueCreateFromPthread(pthread_self());
+ [_exitingThreadsLock lock];
+ NS_DURING
+ {
+ thr = NSMapGet(_exitingThreads, (const void*)selfThread);
+ }
+ NS_HANDLER
+ {
+ [_exitingThreadsLock unlock];
+ DESTROY(selfThread);
+ [localException raise];
+ }
+ NS_ENDHANDLER
+ [_exitingThreadsLock unlock];
+ DESTROY(selfThread);
+ }
if (nil == thr)
{
GSRegisterCurrentThread();
@@ -538,7 +726,6 @@
}
}
-
@implementation NSThread
static void
@@ -662,8 +849,11 @@
* Ensure that the default thread exists.
*/
threadClass = self;
-
+ _exitingThreads = NSCreateMapTable(_boxedPthreadKeyCallBacks,
+ NSObjectMapValueCallBacks, 10);
+ _exitingThreadsLock = [NSLock new];
GSCurrentThread();
+
}
}
@@ -1514,7 +1704,7 @@
modes: commonModes()];
}
-- (void) performSelectorInBackground: (SEL)aSelector
+- (void) performSelectorInBackground: (SEL)aSelector
withObject: (id)anObject
{
[NSThread detachNewThreadSelector: aSelector
Copied: libs/base/trunk/Tests/base/NSThread/late_unregister.m (from r39317,
libs/base/trunk/Tests/base/NSThread/lazy_thread.m)
URL:
http://svn.gna.org/viewcvs/gnustep/libs/base/trunk/Tests/base/NSThread/late_unregister.m?p2=libs/base/trunk/Tests/base/NSThread/late_unregister.m&p1=libs/base/trunk/Tests/base/NSThread/lazy_thread.m&r1=39317&r2=39318&rev=39318&view=diff
==============================================================================
--- libs/base/trunk/Tests/base/NSThread/lazy_thread.m (original)
+++ libs/base/trunk/Tests/base/NSThread/late_unregister.m Fri Jan 29
14:42:07 2016
@@ -1,35 +1,129 @@
#import "ObjectTesting.h"
#import <Foundation/NSThread.h>
+#import <Foundation/NSLock.h>
+#import <Foundation/NSNotification.h>
#include <pthread.h>
-void *thread(void *ignored)
+
+
+
+@interface ThreadExpectation : NSObject <NSLocking>
{
- return [NSThread currentThread];
+ NSCondition *condition;
+ NSThread *origThread;
+ BOOL done;
+ BOOL deallocated;
}
+- (void)onThreadExit: (NSNotification*)n;
+- (BOOL)isDone;
+@end
+
+@implementation ThreadExpectation
+
+- (id)init
+{
+ if (nil == (self = [super init]))
+ {
+ return nil;
+ }
+ condition = [NSCondition new];
+ return self;
+}
+
+
+
+- (void)inThread: (NSThread*)thread
+{
+ /* We explicitly don't retain this so that we can check that it actually says
+ * alive until the notification is sent. That check is implicit since
+ * PASS_EQUAL in the -onThreadExit method will throw or crash if that isn't
+ * the case.
+ */
+ origThread = thread;
+ [[NSNotificationCenter defaultCenter] addObserver: self
+ selector: @selector(onThreadExit:)
+ name:
NSThreadWillExitNotification
+ object: thread];
+}
+
+- (void)onThreadExit: (NSNotification*)thr
+{
+ NSThread *current = [NSThread currentThread];
+
+ PASS_EQUAL(origThread,current,
+ "Correct thread reference can be obtained on exit");
+ [[NSNotificationCenter defaultCenter] removeObserver: self];
+ origThread = nil;
+ [condition lock];
+ done = YES;
+ [condition broadcast];
+ [condition unlock];
+}
+
+- (BOOL)isDone
+{
+ return done;
+}
+
+- (void)waitUntilDate: (NSDate*)date
+{
+ [condition waitUntilDate: date];
+}
+
+- (void)lock
+{
+ [condition lock];
+}
+
+- (void)unlock
+{
+ [condition unlock];
+}
+
+- (void)dealloc
+{
+ DESTROY(condition);
+ [super dealloc];
+}
+@end
+
+void *thread(void *expectation)
+{
+ [(ThreadExpectation*)expectation inThread: [NSThread currentThread]];
+ return nil;
+}
+
+
+
+/**
+ * This test checks whether we can still obtain a reference to the NSThread
+ * object of a thread that is in the process of exiting without an explicit
+ * call to [NSThread exit]. To do this, we pass an expectation object to
+ * a thread created purely using the pthreads API. We then wait on a condition
+ * until the thread exits and posts the NSThreadWillExitNotification. If that
+ * does not happen within 5 seconds, we flag the test as failed.
+ */
int main(void)
{
pthread_t thr;
+ pthread_attr_t attr;
void *ret;
+ pthread_attr_init(&attr);
+ pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+ NSAutoreleasePool *arp = [NSAutoreleasePool new];
+ ThreadExpectation *expectation = [ThreadExpectation new];
+ pthread_create(&thr, &attr, thread, expectation);
- pthread_create(&thr, NULL, thread, NULL);
- pthread_join(thr, &ret);
- PASS(ret != 0, "NSThread lazily created from POSIX thread");
- testHopeful = YES;
- PASS((ret != 0) && (ret != [NSThread mainThread]),
- "Spawned thread is not main thread");
- pthread_create(&thr, NULL, thread, NULL);
- pthread_join(thr, &ret);
- PASS(ret != 0, "NSThread lazily created from POSIX thread");
- PASS((ret != 0) && (ret != [NSThread mainThread]),
- "Spawned thread is not main thread");
-
- NSThread *t = [NSThread currentThread];
- [t setName: @"xxxtestxxx"];
- NSLog(@"Thread description is '%@'", t);
- NSRange r = [[t description] rangeOfString: @"name = xxxtestxxx"];
- PASS(r.length > 0, "thread description contains name");
-
+ NSDate *start = [NSDate date];
+ [expectation lock];
+ while (![expectation isDone] && [start timeIntervalSinceNow] > -5.0f)
+ {
+ [expectation waitUntilDate: [NSDate dateWithTimeIntervalSinceNow: 0.5f]];
+ }
+ PASS([expectation isDone], "Notification for thread exit was sent");
+ [expectation unlock];
+ DESTROY(expectation);
+ DESTROY(arp);
return 0;
}
-
_______________________________________________
Gnustep-cvs mailing list
[email protected]
https://mail.gna.org/listinfo/gnustep-cvs
