macOS Memory bug in Mac app

[barenature]

Hello,

A memory related bug has been bugging me (read the irony) for the past few days and I'd appreciate any pointers to help solve this issue. Only a very small percentage of my users seems to be affected by it and I cannot reproduce it on my system. The crash report below is from one of my users.

The crash report looks like this and the method that leads up to the crash is pasted below as well. Some other people I talked to about this thought in the direction of memory being trashed or overwritten. When running my app with Guard Malloc does not tell me anything more about the issue. I'd appreciate your help and time!

Process:         Application [81831]
Path:            /Applications/myapplication.app/Contents/MacOS/Application
Identifier:      com.barenature.myapplication
Version:         1.1 (1.1)
Code Type:       X86-64 (Native)
Parent Process:  launchd [181]

Date/Time:       2011-02-14 20:45:15.446 -0700
OS Version:      Mac OS X 10.6.6 (10J567)
Report Version:  6

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000000
Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Application Specific Information:
objc[81831]: garbage collection is ON

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread

0   libSystem.B.dylib                  0x00007fffffe00847 __memcpy + 167
1   libauto.dylib                      0x00007fff82718170 auto_zone_write_barrier_memmove + 96
2   libauto.dylib                      0x00007fff8271916e auto_realloc(_malloc_zone_t*, void*, unsigned long) + 878
3   libSystem.B.dylib                  0x00007fff8346e0db malloc_zone_realloc + 92
4   com.apple.Foundation               0x00007fff83169836 _NSMutableDataGrowBytes + 652
5   com.apple.Foundation               0x00007fff83169513 -[NSConcreteMutableData appendBytes:length:] + 101
6   ...barenature.myapplication        0x000000010000b9cd -[Connection stream:handleEvent:] + 376
7   com.apple.CoreFoundation          0x00007fff85742373 _signalEventSync + 115

- (void)stream:(NSStream *)stream handleEvent:(NSStreamEvent)eventCode {

    switch(eventCode) {
          case NSStreamEventHasSpaceAvailable: {

               if (stream == outputStream) {
                    [self writeBufferToStream];
               }

               break;
          }

          case NSStreamEventOpenCompleted:

            if (stream == inputStream) {
               readReady = YES;

            } else {
               writeReady = YES;
            }

            if ([self isReadyForUse] && [delegate respondsToSelector:@selector(connectionReadyForUse:)])
               [delegate connectionReadyForUse:self];
            break;

        case NSStreamEventHasBytesAvailable: {

               if (stream == inputStream) {

                    int bytesRead = 0;

                    static uint8_t buffer[kBufferSize];
                    bytesRead = [inputStream read:buffer maxLength:sizeof(buffer)];
                    [inBuffer appendBytes:buffer length:bytesRead];

                    //** Process buffer contents **//

                    BOOL safe = YES;

                    while (safe) {
                         if (inSize <= 0) {
                              if ([inBuffer length] >= sizeof(uint64_t)) {
                                   memcpy(&inSize, [inBuffer bytes], sizeof(uint64_t));
                                   NSRange rangeToDelete = {0, sizeof(uint64_t)};
                                   [inBuffer replaceBytesInRange:rangeToDelete withBytes:NULL length:0];

                              } else {
                                   break;
                              }
                         }


                         if (inSize > 0) {
                              if ([inBuffer length] >= inSize) {
                                   NSMutableData *packetData = [NSMutableData dataWithBytes:[inBuffer bytes] length:inSize];                                        
                                   [delegate connection:self receivedData:packetData];

                                   safe = NO;


                                   NSRange rangeToDelete = {0, inSize};
                                   [inBuffer replaceBytesInRange:rangeToDelete withBytes:NULL length:0];
                                   inSize = 0;
                              } else {
                                   break;
                              }
                         } else {
                              break;
                         }
                    }
               }
            break;
               }

        case NSStreamEventErrorOccurred: {

            NSError *theError = [stream streamError];
            if (stream == inputStream)
                if (delegate && [delegate respondsToSelector:@selector(connection:encounteredReadError:)])
                    [delegate connection:self encounteredReadError:theError];
                else{
                    if (delegate && [delegate respondsToSelector:@selector(connection:encounteredWriteError:)])
                        [delegate connection:self encounteredWriteError:theError];   
                }

            break;
        }

        case NSStreamEventEndEncountered: {
            if (delegate && [delegate respondsToSelector:@selector(connectionDisconnected:)])
                [delegate connectionDisconnected:self];

            readReady = NO;
            writeReady = NO;
            break;
          }

        default:
            break;
    }
}

 

[gnasher729]

NSLog is your friend. I'd use it to print which amounts of bytes you handle at each point (typically the value of inSize). And somehow I think you will run into trouble when a buffer you receive contains only parts of the eight byte size information.

 

[chown33]

                    [COLOR="Red"]static uint8_t buffer[kBufferSize];[/COLOR]
                    bytesRead = [inputStream read:buffer maxLength:sizeof(buffer)];
                    [inBuffer appendBytes:buffer length:bytesRead];

The red-hilited code looks to me like a concurrency bomb waiting to explode.

That's a static buffer. It's shared by all instances of the class. I see no locks or other mechanisms to prevent or coordinate concurrent access by multiple objects or threads.

I see no reason why this buffer should be static, and I can think of several reasons why it shouldn't be.

 

[kainjow]

read:maxLength: returns an NSInteger and "A negative number means that the operation failed" so you need to check for that.

This produces an identical stack trace (with GC enabled):

[color=#643820]#import [/color][color=#c41a16]<Foundation/Foundation.h>[/color][color=#643820]
[/color]
[color=#aa0d91]int[/color] main ([color=#aa0d91]int[/color] argc, [color=#aa0d91]const[/color] [color=#aa0d91]char[/color] * argv[]) {
    [color=#5c2699]NSMutableData[/color] *d = [[color=#5c2699]NSMutableData[/color] [color=#2e0d6e]data[/color]];
    [color=#5c2699]uint32_t[/color] i = [color=#1c00cf]42[/color];
    [d [color=#2e0d6e]appendBytes[/color]:&i [color=#2e0d6e]length[/color]:-[color=#1c00cf]1[/color]];
    [color=#aa0d91]return[/color] [color=#1c00cf]0[/color];
}

 

[barenature]

Thank you all for your input! Much appreciated.

I have been running some tests with guard malloc and a global breakpoint at "malloc_error". With guard malloc enabled, I can sometimes reproduce the error some of my users are having. When this happens, I get this message in the console.

GuardMalloc[MyApplication-16628]: Attempting excessively large memory allocation:  1073741822 bytes
GuardMalloc[MyApplication-16628]: If you really wanted to allocate so much memory, launch your executable with the environment variable MALLOC_PERMIT_INSANE_REQUESTS set to any value to circumvent this check.
GuardMalloc[MyApplication-16628]: Explicitly trapping into debugger!!!

Is the reason that "bytesRead" is excessively large because it acutally returns "-1" (as kainjow mentioned) and that the -appendBytes:length: method expects a positive value? Or is this due to another problem, for example, what gnasher729 or chown33 suggested?

The debugger indicates that the error occurs when the buffer is being written to inBuffer (see code snippet):

[inBuffer appendBytes:buffer length:bytesRead];

@chown33: What do you mean with "a concurrency bomb"? What is a better approach in your opinion to capture the data form the inputStream? For your information, kBufferSize equals to 1024 in this case.

I have the feeling that we're almost there to finally squash this bug.

Thanks!

 

[chown33]

@chown33: What do you mean with "a concurrency bomb"? What is a better approach in your opinion to capture the data form the inputStream? For your information, kBufferSize equals to 1024 in this case.

I mean you have a problem waiting to happen, whenever there is concurrent use of the single shared static buffer.

If there is more than one instance at the same time (i.e. concurrent instances) and they both use the same buffer (concurrent access), then some data will be overwritten. The key error is that there is only one buffer. It can only hold one set of bytes. If two or more reads go into the buffer, then only one set of bytes will be correct. All other object instances using the same buffer will not be reading the bytes they put into the buffer.

I suggest using a local buffer. Remove the 'static' keyword and you will have a local buffer. Every instance will then get its own local buffer and there will not be any sharing between instances.

I can't see why anyone would intentionally add the 'static' keyword. This seems like you copied and pasted code without understanding it.

 

[barenature]

That makes sense. Thanks for sharing this!

 

[chown33]

I have been running some tests with guard malloc and a global breakpoint at "malloc_error". With guard malloc enabled, I can sometimes reproduce the error some of my users are having.

The requested size is 1073741822. That's not -1. In fact, it's 1GiB - 2.

It's possible that something is limiting a request of -1 to the value 1GiB - 2. You could make a test case and see what happens, like kainjow did.

The actual cause could be anything. I suggest fixing the bugs you know about one at a time, then rerunning under your guard malloc test regime after each fix. If the same error crops up, then you've either botched the bug-fix or the fixed bug wasn't causal.