From 675b59127d0ec5ca7ae54647d90879b245f2d5c7 Mon Sep 17 00:00:00 2001 From: "gtm.daemon" Date: Thu, 3 Feb 2011 18:30:16 +0000 Subject: [Author: aharper] Fix interaction of GTMFileSystemKqueue when the runloop isn't spinning. Also, don't count on kCFSocketReadCallBack to keep calling us, empty the queue whenever we can. DELTA=93 (84 added, 0 deleted, 9 changed) R=dmaclach,thomasvl --- Foundation/GTMFileSystemKQueueTest.m | 168 ++++++++++++++++++++++++----------- 1 file changed, 117 insertions(+), 51 deletions(-) (limited to 'Foundation/GTMFileSystemKQueueTest.m') diff --git a/Foundation/GTMFileSystemKQueueTest.m b/Foundation/GTMFileSystemKQueueTest.m index a64fd2b..d80f161 100644 --- a/Foundation/GTMFileSystemKQueueTest.m +++ b/Foundation/GTMFileSystemKQueueTest.m @@ -6,9 +6,9 @@ // Licensed under the Apache License, Version 2.0 (the "License"); you may not // use this file except in compliance with the License. You may obtain a copy // of the License at -// +// // http://www.apache.org/licenses/LICENSE-2.0 -// +// // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, WITHOUT // WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -18,8 +18,16 @@ #import "GTMSenTestCase.h" #import "GTMFileSystemKQueue.h" +#import "GTMGarbageCollection.h" #import "GTMUnitTestDevLog.h" + +// Private methods of GTMFileSystemKQueue we use for some tests +@interface GTMFileSystemKQueue (PrivateMethods) +- (void)unregisterWithKQueue; +@end + + @interface GTMFileSystemKQueueTest : GTMTestCase { @private NSString *testPath_; @@ -38,13 +46,13 @@ @implementation GTMFSKQTestHelper -- (void)callbackForQueue:(GTMFileSystemKQueue *)queue +- (void)callbackForQueue:(GTMFileSystemKQueue *)queue events:(GTMFileSystemKQueueEvents)event { // Can't use standard ST macros here because our helper // is not a subclass of GTMTestCase. This is intentional. if (queue != queue_) { NSString *file = [NSString stringWithUTF8String:__FILE__]; - NSException *exception + NSException *exception = [NSException failureInEqualityBetweenObject:queue andObject:queue_ inFile:file @@ -52,7 +60,7 @@ withDescription:nil]; [exception raise]; } - + if (event & kGTMFileSystemKQueueWriteEvent) { ++writes_; } @@ -115,7 +123,7 @@ [fm removeFileAtPath:testPath_ handler:nil]; [fm removeFileAtPath:testPath2_ handler:nil]; #endif // MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 - + [testPath_ release]; testPath_ = nil; [testPath2_ release]; @@ -126,13 +134,13 @@ GTMFileSystemKQueue *testKQ; GTMFSKQTestHelper *helper = [[[GTMFSKQTestHelper alloc] init] autorelease]; STAssertNotNil(helper, nil); - + // init should fail [GTMUnitTestDevLog expectString:@"Don't call init, use " @"initWithPath:forEvents:acrossReplace:target:action:"]; testKQ = [[[GTMFileSystemKQueue alloc] init] autorelease]; STAssertNil(testKQ, nil); - + // no path testKQ = [[[GTMFileSystemKQueue alloc] initWithPath:nil @@ -141,7 +149,7 @@ target:helper action:@selector(callbackForQueue:events:)] autorelease]; STAssertNil(testKQ, nil); - + // not events testKQ = [[[GTMFileSystemKQueue alloc] initWithPath:@"/var/log/system.log" @@ -150,7 +158,7 @@ target:helper action:@selector(callbackForQueue:events:)] autorelease]; STAssertNil(testKQ, nil); - + // no target testKQ = [[[GTMFileSystemKQueue alloc] initWithPath:@"/var/log/system.log" @@ -159,7 +167,7 @@ target:nil action:@selector(callbackForQueue:events:)] autorelease]; STAssertNil(testKQ, nil); - + // no handler testKQ = [[[GTMFileSystemKQueue alloc] initWithPath:@"/var/log/system.log" @@ -169,7 +177,7 @@ action:nil] autorelease]; STAssertNil(testKQ, nil); - + // path that doesn't exist testKQ = [[[GTMFileSystemKQueue alloc] initWithPath:@"/path/that/does/not/exist" @@ -191,15 +199,15 @@ } - (void)testWriteAndDelete { - + NSFileManager *fm = [NSFileManager defaultManager]; GTMFSKQTestHelper *helper = [[[GTMFSKQTestHelper alloc] init] autorelease]; STAssertNotNil(helper, nil); - + STAssertTrue([fm createFileAtPath:testPath_ contents:nil attributes:nil], nil); NSFileHandle *testFH = [NSFileHandle fileHandleForWritingAtPath:testPath_]; STAssertNotNil(testFH, nil); - + // Start monitoring the file GTMFileSystemKQueue *testKQ = [[GTMFileSystemKQueue alloc] initWithPath:testPath_ @@ -210,14 +218,14 @@ STAssertNotNil(testKQ, nil); STAssertEqualObjects([testKQ path], testPath_, nil); [helper setKQueue:testKQ]; - + // Write to the file [testFH writeData:[@"doh!" dataUsingEncoding:NSUnicodeStringEncoding]]; - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 1, nil); - + // Close and delete [testFH closeFile]; #if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 @@ -226,34 +234,34 @@ #else STAssertTrue([fm removeFileAtPath:testPath_ handler:nil], nil); #endif // MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 - + [self spinForEvents:helper]; STAssertEquals([helper totals], 2, nil); // Clean up the kqueue [testKQ release]; testKQ = nil; - + STAssertEquals([helper writes], 1, nil); STAssertEquals([helper deletes], 1, nil); STAssertEquals([helper renames], 0, nil); } - (void)testWriteAndDeleteAndWrite { - + // One will pass YES to |acrossReplace|, the other will pass NO. - + NSFileManager *fm = [NSFileManager defaultManager]; GTMFSKQTestHelper *helper = [[[GTMFSKQTestHelper alloc] init] autorelease]; STAssertNotNil(helper, nil); GTMFSKQTestHelper *helper2 = [[[GTMFSKQTestHelper alloc] init] autorelease]; STAssertNotNil(helper, nil); - + // Create a temp file path STAssertTrue([fm createFileAtPath:testPath_ contents:nil attributes:nil], nil); NSFileHandle *testFH = [NSFileHandle fileHandleForWritingAtPath:testPath_]; STAssertNotNil(testFH, nil); - + // Start monitoring the file GTMFileSystemKQueue *testKQ = [[GTMFileSystemKQueue alloc] initWithPath:testPath_ @@ -264,7 +272,7 @@ STAssertNotNil(testKQ, nil); STAssertEqualObjects([testKQ path], testPath_, nil); [helper setKQueue:testKQ]; - + GTMFileSystemKQueue *testKQ2 = [[GTMFileSystemKQueue alloc] initWithPath:testPath_ forEvents:kGTMFileSystemKQueueAllEvents @@ -274,7 +282,7 @@ STAssertNotNil(testKQ2, nil); STAssertEqualObjects([testKQ2 path], testPath_, nil); [helper2 setKQueue:testKQ2]; - + // Write to the file [testFH writeData:[@"doh!" dataUsingEncoding:NSUnicodeStringEncoding]]; @@ -297,7 +305,7 @@ testFH = [NSFileHandle fileHandleForWritingAtPath:testPath_]; STAssertNotNil(testFH, nil); [testFH writeData:[@"ha!" dataUsingEncoding:NSUnicodeStringEncoding]]; - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 2, nil); @@ -305,12 +313,12 @@ // Write to it again [testFH writeData:[@"continued..." dataUsingEncoding:NSUnicodeStringEncoding]]; - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 3, nil); STAssertEquals([helper2 totals], 2, nil); - + // Close and delete [testFH closeFile]; #if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 @@ -318,18 +326,18 @@ #else STAssertTrue([fm removeFileAtPath:testPath_ handler:nil], nil); #endif // MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 4, nil); STAssertEquals([helper2 totals], 2, nil); - + // Clean up the kqueue [testKQ release]; testKQ = nil; [testKQ2 release]; testKQ2 = nil; - + STAssertEquals([helper writes], 2, nil); STAssertEquals([helper deletes], 2, nil); STAssertEquals([helper renames], 0, nil); @@ -339,7 +347,7 @@ } - (void)testWriteAndRenameAndWrite { - + // One will pass YES to |acrossReplace|, the other will pass NO. NSFileManager *fm = [NSFileManager defaultManager]; @@ -347,12 +355,12 @@ STAssertNotNil(helper, nil); GTMFSKQTestHelper *helper2 = [[[GTMFSKQTestHelper alloc] init] autorelease]; STAssertNotNil(helper2, nil); - + // Create a temp file path STAssertTrue([fm createFileAtPath:testPath_ contents:nil attributes:nil], nil); NSFileHandle *testFH = [NSFileHandle fileHandleForWritingAtPath:testPath_]; STAssertNotNil(testFH, nil); - + // Start monitoring the file GTMFileSystemKQueue *testKQ = [[GTMFileSystemKQueue alloc] initWithPath:testPath_ @@ -363,7 +371,7 @@ STAssertNotNil(testKQ, nil); STAssertEqualObjects([testKQ path], testPath_, nil); [helper setKQueue:testKQ]; - + GTMFileSystemKQueue *testKQ2 = [[GTMFileSystemKQueue alloc] initWithPath:testPath_ forEvents:kGTMFileSystemKQueueAllEvents @@ -373,43 +381,43 @@ STAssertNotNil(testKQ2, nil); STAssertEqualObjects([testKQ2 path], testPath_, nil); [helper2 setKQueue:testKQ2]; - + // Write to the file [testFH writeData:[@"doh!" dataUsingEncoding:NSUnicodeStringEncoding]]; - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 1, nil); STAssertEquals([helper2 totals], 1, nil); - + // Move it and create the file again #if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 NSError *error = nil; - STAssertTrue([fm moveItemAtPath:testPath_ toPath:testPath2_ error:&error], + STAssertTrue([fm moveItemAtPath:testPath_ toPath:testPath2_ error:&error], @"Error: %@", error); #else STAssertTrue([fm movePath:testPath_ toPath:testPath2_ handler:nil], nil); #endif // MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 - + STAssertTrue([fm createFileAtPath:testPath_ contents:nil attributes:nil], nil); NSFileHandle *testFHPrime = [NSFileHandle fileHandleForWritingAtPath:testPath_]; STAssertNotNil(testFHPrime, nil); [testFHPrime writeData:[@"eh?" dataUsingEncoding:NSUnicodeStringEncoding]]; - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 2, nil); STAssertEquals([helper2 totals], 2, nil); - + // Write to the new file [testFHPrime writeData:[@"continue..." dataUsingEncoding:NSUnicodeStringEncoding]]; - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 3, nil); STAssertEquals([helper2 totals], 2, nil); - + // Write to the old file [testFH writeData:[@"continue old..." dataUsingEncoding:NSUnicodeStringEncoding]]; @@ -417,7 +425,7 @@ [self spinForEvents:helper]; STAssertEquals([helper totals], 3, nil); STAssertEquals([helper2 totals], 3, nil); - + // and now close old [testFH closeFile]; #if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 @@ -425,12 +433,12 @@ #else STAssertTrue([fm removeFileAtPath:testPath2_ handler:nil], nil); #endif // MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 3, nil); STAssertEquals([helper2 totals], 4, nil); - + // and now close new [testFHPrime closeFile]; #if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 @@ -438,18 +446,18 @@ #else STAssertTrue([fm removeFileAtPath:testPath_ handler:nil], nil); #endif // MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 - + // Spin the runloop for a second so that the helper callbacks fire [self spinForEvents:helper]; STAssertEquals([helper totals], 4, nil); STAssertEquals([helper2 totals], 4, nil); - + // Clean up the kqueue [testKQ release]; testKQ = nil; [testKQ2 release]; testKQ2 = nil; - + STAssertEquals([helper writes], 2, nil); STAssertEquals([helper deletes], 1, nil); STAssertEquals([helper renames], 1, nil); @@ -458,4 +466,62 @@ STAssertEquals([helper2 renames], 1, nil); } +- (void)testNoSpinHang { + // This case tests a specific historically problematic interaction of + // GTMFileSystemKQueue and the runloop. GTMFileSystemKQueue uses the CFSocket + // notifications (and thus the runloop) for monitoring, however, you can + // dealloc the instance (and thus unregister the underlying kevent descriptor) + // prior to any runloop spin. The unregister removes the pending notifications + // from the monitored main kqueue file descriptor that CFSocket has previously + // noticed but not yet called back. At that point a kevent() call in the + // socket callback without a timeout would hang the runloop. + + // Warn this may hang + NSLog(@"%s on failure this will hang.", __PRETTY_FUNCTION__); + + NSFileManager *fm = [NSFileManager defaultManager]; + GTMFSKQTestHelper *helper = [[[GTMFSKQTestHelper alloc] init] autorelease]; + STAssertNotNil(helper, nil); + STAssertTrue([fm createFileAtPath:testPath_ contents:nil attributes:nil], nil); + NSFileHandle *testFH = [NSFileHandle fileHandleForWritingAtPath:testPath_]; + STAssertNotNil(testFH, nil); + + // Start monitoring the file + GTMFileSystemKQueue *testKQ + = [[GTMFileSystemKQueue alloc] initWithPath:testPath_ + forEvents:kGTMFileSystemKQueueAllEvents + acrossReplace:YES + target:helper + action:@selector(callbackForQueue:events:)]; + STAssertNotNil(testKQ, nil); + STAssertEqualObjects([testKQ path], testPath_, nil); + [helper setKQueue:testKQ]; + + // Write to the file + [testFH writeData:[@"doh!" dataUsingEncoding:NSUnicodeStringEncoding]]; + // Close and delete + [testFH closeFile]; +#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 + NSError *error = nil; + STAssertTrue([fm removeItemAtPath:testPath_ error:&error], @"Err: %@", error); +#else + STAssertTrue([fm removeFileAtPath:testPath_ handler:nil], nil); +#endif // MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5 + + // Now destroy the queue, with events outstanding from the CFSocket, but + // unconsumed. For GC this involves us using a private method since |helper| + // still has a reference. For non-GC we'll force the release. + if (GTMIsGarbageCollectionEnabled()) { + [testKQ unregisterWithKQueue]; + } else { + STAssertEquals([testKQ retainCount], (NSUInteger)1, nil); + [testKQ release]; + testKQ = nil; + } + + // Spin the runloop, no events were delivered (and we should not hang) + [self spinForEvents:helper]; + STAssertEquals([helper totals], 0, nil); +} + @end -- cgit v1.2.3