Objective C thread dieing with EXC_BAD_ACCESS on stringWithFormat

Discussion in 'Mac Programming' started by Aumnayan, Dec 4, 2010.

  1. Aumnayan, Dec 4, 2010
    Last edited: Dec 4, 2010

    Aumnayan
    Expand Collapse
    macrumors newbie

    Joined:
    Dec 4, 2010
    #1
    I'm running up against an interesting problem that I've been unable to find a solution for. The below code dies when it reaches the string formatting line sptr = [NSString...] with an EXC_BAD_ACCESS signal. curFile and totalFiles are both set previously, I've even hardcoded them to various values, and tried various diffrent strings and the thread always dies at the first instance of [NSString stringWithFormat:#]. It dies in the same way if I use [[NSString alloc] initWithFormat:#].

    Can anyone point me at something I'm doing wrong? (Garbage collection in enabled as FYI).

    Code:
    <On a button push>
    [NSThread detachNewThreadSelector:@selector(CreationThread) toTarget:self withObject:nil];
    
    - (void)     CreationThread
    {
    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
    .
    .
    .
    NSString *sptr = [NSString stringWithFormat:@"%i of %i", curFile, totalFiles];
    [progressLabel setStringValue:sptr];
    sptr = nil;
    .
    .
    .
    [pool drain]
    }
    
    
    [EDIT 1:57pm PST]
    A few additional pieces of information: If I call the function CreationThread directly instead of instantiating a new thread the code works as is, with no modification. curFile and TotalFiles are local to CreationThread. I've hardcoded both variables to 0 prior to the string formatting and it's still causing an EXEC_BAD_ACCESS signal.

    Nixing the multithreading isn't really a desirable solution. Thus the post.
     
  2. lee1210
    Expand Collapse
    macrumors 68040

    lee1210

    Joined:
    Jan 10, 2005
    Location:
    Dallas, TX
    #2
    Have you taken the exact code you provided, stripped out the setStringValue, put it in main for a new program, with curFiles and totalFiles declared and set? If not do that. Does it still crash? If so, post that code. If not, what varies between that and your full program?

    -Lee
     
  3. Aumnayan
    Expand Collapse
    thread starter macrumors newbie

    Joined:
    Dec 4, 2010
    #3
    Taking it out of the thread, and calling the function directly from the button push the code works as is, with no modification.
     
  4. kainjow, Dec 4, 2010
    Last edited: Dec 4, 2010

    kainjow
    Expand Collapse
    Moderator emeritus

    kainjow

    Joined:
    Jun 15, 2000
    #4
    1. Since you have GC enabled, remove the autorelease pools.

    2. Assuming progressLabel is an NSTextField or some other object part of AppKit, you cannot and should not access that from a thread other than main. You need to use performSelectorOnMainThread to set its stringValue. Something like this:
    Code:
    [progressLabel performSelectorOnMainThread:@selector(setStringValue:) withObject:sptr waitUntilDone:NO];
     
  5. Aumnayan, Dec 9, 2010
    Last edited: Dec 9, 2010

    Aumnayan
    Expand Collapse
    thread starter macrumors newbie

    Joined:
    Dec 4, 2010
    #5
    Thanks for the reply kainjow, I've reworked the code such that the thread isn't accessing any of the GUI elements, however I can't seam to get away from the EXC_BAD_ACCESS. I believe there is something rather basic that I am doing wrong, however for the life of me I can't figure it out.

    Below is the relevant code snippets, and the observations made through GDB.

    Code:
    @interface GUI : NSObject {
    
    NSString                           *placeholder1;
    NSString                           *placeholder2;
    IBOutlet NSTextField           *textField1;
    IBOutlet NSTextField           *textField2;
    IBOutlet NSButton              *StartButton;
      
    }
    
    - (IBAction) Button:(id)sender;
    - (void) threadMain;
    - (void) fun1:(NSString*)str1: (int)y: (int)x: (NSString*) str2;
    @end
    
    @implementation GUI
    - (IBAction) StartMapGen:(id) sender
    {
      placeholder1 = [[NSString alloc] initWithString:[textField1 stringValue]];
      placeholder2 = [[NSString alloc] initWithString:[textField2 stringValue]];
      [GenerateButton setEnabled:FALSE];
      [NSThread detachNewThreadSelector:@selector(threadMain) 
                                 toTarget:self 
                                 withObject:nil];
    }
    
    - (void) threadMain
    {
      NSString *localpath = [[NSString alloc] initWithString:placeholder];
      DIR *pDir;
      struct dirent *entry;
    
      /* Leaving out code that reads through directory & fills entry */
    
      int x, y;
      x = /*someval*/
      y = /*someval*/
      NSString *inPath = [[NSString alloc] initWithFormat:@"%@/%s",
                                      placeholder2, entry->d_name];
    
      // At this point x, y, inPath, and localPath all evaluate properly in GDB.
      // aka, things work to this point.
      [self fun1:inPath: y: x: localPath];
    }
    
    - (void) fun1:(NSString*)str1: (int)y: (int)x: (NSString*) str2
    {
      /* At this point, str1, and str2 are flaged as invalid in gdb though x & y are valid. They are
          pointing at the correct memory, it's like the garbage collector has
          deallocated the memory between sending the message and actually
          processing it... which leave me with the question... huh? */
    
      NSString *newStr = [[NSString alloc] initWithFormat:@"%@/00%3i%3i", str2, y, x];  // EXEC_BAD_ACCESS, see previous comment.
    }
    @end
    
    Any help would be appreciated.
     
  6. whooleytoo
    Expand Collapse
    macrumors 603

    whooleytoo

    Joined:
    Aug 2, 2002
    Location:
    Cork, Ireland.
    #6
    Is that code actually compiling? I'd have thought how fun1 is declared and called is wrong?

    Should it be declared:
    Code:
    - (void) fun1:(NSString*)inFile lat: (int)lat lon:(int)lon outPath:(NSString*) outPath;
    
    and
    Code:
    [self fun1:inPath: y: x: localPath];
    
    should surely be:
    Code:
    [self fun1: inPath lat:x lon:y outPath:localPath];
     
  7. Aumnayan
    Expand Collapse
    thread starter macrumors newbie

    Joined:
    Dec 4, 2010
    #7
    I had an error in the snippet, cased by me copying and pasting the method definition and hand typing the methods declaration. I've changed them to match in the code above.

    Yes it is compiling fine. It's my understanding (albeit, I've only just started messing with objective c) that naming the variables in that way is optional. I could easily have misread the reference talking about it however. Regardless, changing fun1 to the syntax you specified leaves doesn't change the result of the application. EXEC_BAD_ACCESS as soon as I enter into it.
     
  8. jared_kipe
    Expand Collapse
    macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #8
    You can in fact compile and run functions declared that way. Its sorta like how C functions don't have descriptions only names and types of input variables.
    Code:
    void fun(NSString *str1, int y, int x, NSString *str2);
    - (void) fun1: (NSString *)str1 : (int)y : (int)x : (NSString *) str2;
    
    The second prototype would be called like this.
    Code:
    [mySomething fun1: @"" : 10 : 12 : @""];
    
    I would never ever declare or call a function like this, but the description elements are not necessary.
     
  9. jared_kipe, Dec 9, 2010
    Last edited: Dec 9, 2010

    jared_kipe
    Expand Collapse
    macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #9
    EXEC_BAD_ACCESS means you sent a selector to an address and there was nothing there to receive it, namely no class/object.

    Since the only call you posted to do such a thing would be NSString *newStr = [[NSString alloc] initWithFormat:mad:"%@/00%3i%3i", outPath, y, x];

    Implicitly calling -description on outPath, I'd suggest figuring out what or where outPath is. Because it is not an object at this moment, or has been released.

    Check that GC is actually turned on. Enable zombies. Turn off garbage collection and try to debug why this object isn't actually around.

    EDIT: On second thought, what is outPath?! It doesn't seem to be declared anywhere. And is "struct dirent *entry" heap allocated or something, why be a pointer and not local?
     
  10. whooleytoo
    Expand Collapse
    macrumors 603

    whooleytoo

    Joined:
    Aug 2, 2002
    Location:
    Cork, Ireland.
    #10
    Didn't know that, thanks guys.

    Where is outPath being declared? I don't see it anywhere in the code.

    Also - you might check is 'self' is valid in fun1.
     
  11. lee1210
    Expand Collapse
    macrumors 68040

    lee1210

    Joined:
    Jan 10, 2005
    Location:
    Dallas, TX
    #11
    His spacing makes it much worse to understand:
    Code:
    - (void) fun1:(NSString*)str1  :(int)y  :(int)x   :(NSString*)str2;
    
     [self fun1:inPath  :y  :x  :localPath];
    
    If it was written like that it might still look bad, but you could at least see what's what. Before the spacing made it look like there were labels for the parameters with nothing being passed to them.

    As for the dirent thing, there are system calls that return this type such as readdir, but the OP didn't include that code for whatever reason.

    -Lee
     
  12. jared_kipe
    Expand Collapse
    macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #12
    Ahh I've never used dirent.h before and assumed it was some struct specific to his program.

    I was scanning up through it visually and saw entry->d_name, and immediately thought maybe it should be entry.d_name then saw it was in fact declared as a pointer. Given the error has to do with accessing memory I thought I might as well ask.
     
  13. ulbador
    Expand Collapse
    macrumors 68000

    ulbador

    Joined:
    Feb 11, 2010
    #13
    Turn on breakpoints, and type bt in the debug console after it dies. That should give you a pretty good idea on what it is dying on.
     
  14. Aumnayan
    Expand Collapse
    thread starter macrumors newbie

    Joined:
    Dec 4, 2010
    #14
    Sorry another copy/past to typing mess up with regards to outPath. outPath was supposed to be str2 in the snippet.

    *entry is get set to the return of a readdir() call. I left out the majority of code prior to the call to fun1, save declarations & setting of variables being passed into fun1. Simply put it's lengthy, and the whole point is to scan through several thousand files parsing the information necessary for fun1 to operate. Prior to implementing fun1, I've verified that this aspect of the program works by simply scanning through all files and verifying the ouputs of str1, str2, x, and y. So it wasn't included.

    Prior to call to fun1, I've verified the contents of x, y, str1, and str2 to be valid with output similar to:

    str1 = "/Users/<uname>/Desktop/tmp"
    x = 102
    y = 87
    str2 = "/Users/<uname>/Desktop/output"

    I'll continue debuggin this when I get home from work, and perform the suggestions already posted, and any others that come in. Thanks for looking at this.
     
  15. Sydde, Dec 9, 2010
    Last edited: Dec 9, 2010

    Sydde
    Expand Collapse
    macrumors 68020

    Sydde

    Joined:
    Aug 17, 2009
    #15
    NSThread documentation states the the target selector "must take only one argument". Therefore, I would suggest you declare

    Code:
    -(void)threadMain:(id)unused
    I believe your thread method does have to take an argument, not that you are required to do anything with it. This may have something to do with thefact that your code runs fine on the main thread.

    ( Of course this also means the argument to the detach method needs to be "@selector( threadMain: )" )
     
  16. kainjow
    Expand Collapse
    Moderator emeritus

    kainjow

    Joined:
    Jun 15, 2000
    #16
    ^ I don't think that matters. I've never had problems with NSThread and a selector that doesn't take any arguments.
     
  17. Sydde
    Expand Collapse
    macrumors 68020

    Sydde

    Joined:
    Aug 17, 2009
    #17
    No, you are probably right. If it gives you no trouble, it probably does not matter.

    What I am seeing, however, is that the initWithString: method does not seem to do anything, by which I mean it is no different than as simple assignment statement.
    Code:
    placeholder1 = [[NSString alloc] initWithString:[textField1 stringValue]];
    will produce exactly the same result as
    Code:
    placeholder1 = [textField1 stringValue];
    (this is normal behavior for immutable objects: copy often simply returns the same object with an incremented retain count, in a ref counted environment, because why bother making a copy of something that cannot change).

    AppKit objects tend to be notoriously not-thread-safe. Between the time you assign the value and the start of the thread (especially given that there is another AppKit call between), the text fields might spontaneously, for reasons that apparently make sense to Apple's system engineers, decide to change the object that represents the string value. You are losing the objects because the text fields are destroying them, and that is the only reference you have to them. You could use initWithFormat: or mutableCopy or some other method that will preserve the value. Or you could @synthesize a retained @property, though I am not entirely certain that would fix the issue.
     
  18. Aumnayan
    Expand Collapse
    thread starter macrumors newbie

    Joined:
    Dec 4, 2010
    #18
    I disabled the GC, enabled zombies, and gave it a run. I've included the debugger output & back trace. Its showing pretty much what I've been stateing, but debugger output's more consise then english. ;)

    The problem I'm encountering isn't starting the thread. The thread seams to work just fine without taking an argument. The problem stems from sending self the fun1 message/signal/(whatever the obj-c term is for this that I'm spacing on at the moment) from within the thread. Line 145 actually traces to the open bracket ('{') for fun1's implementation.



    Running…
    unable to read unknown load command 0x22
    2010-12-09 12:43:25.014 Parser[25802:5903] *** _NSAutoreleaseNoPool(): Object 0x1630e210 of class NSCFData autoreleased with no pool in place - just leaking
    Stack: (0x9098ff4f 0x9089c432 0x908b0b25 0x908b0701 0x2332 0x908a2dfd 0x908a29a4 0x978cd155 0x978cd012)
    [Switching to process 25802 thread 0x5903]
    [Switching to process 25802 thread 0x5903]
    Current language: auto; currently objective-c
    Program received signal: “EXC_BAD_ACCESS”.
    (gdb) bt


    #0 0x00002632 in -[fun1:y:x:str2:] (self=0x13e4d0, _cmd=0x2ec8, str1=0x1630f450, y=80, x=250, str2=0x1630a9c0) at /Parser/GUI.m:145
    #1 0x000025e3 in -[GUI threadMain] (self=0x13e4d0, _cmd=0x2eb8) at /Parser/GUI.m:134
    #2 0x908a2dfd in -[NSThread main] ()
    #3 0x908a29a4 in __NSThread__main__ ()
    #4 0x978cd155 in _pthread_start ()
    #5 0x978cd012 in thread_start ()
     
  19. Aumnayan, Dec 9, 2010
    Last edited: Dec 9, 2010

    Aumnayan
    Expand Collapse
    thread starter macrumors newbie

    Joined:
    Dec 4, 2010
    #19
    I was unaware of this, I was thinking that by the call:

    Code:
    placeholder1 = [[NSString alloc] initWithString:[textField1 stringValue]];
    it would be instantiating another instance of that string, and not simply point to the user-modifiable string contained in textField1, thus breaking the thread to appkit link.

    A previous incantation of this code (between my initial post, and the code snippet that is presently displayed) I was using the code:

    Code:
    placeholder1 = [NSString stringWithFormat:@"%@", [textField1 stringValue]];
    placeholder2 = [NSString stringWithFormat:@"%@", [textField2 stringValue]];
    
    However, the application was crashing in the same fashion/place.
     
  20. vocaro
    Expand Collapse
    macrumors regular

    Joined:
    Mar 5, 2004
    #20
    [textField1 stringValue] returns an NSString, which is immutable. Even if the user changes the text in the field, that won't affect the value of your variable.

    What happens if you inline the function, replacing the call to fun1 with its body? Does the program still crash?
     
  21. Sydde
    Expand Collapse
    macrumors 68020

    Sydde

    Joined:
    Aug 17, 2009
    #21
    Bear in mind that Cocoa is designed to be as efficient as practical, even if it confuses programmers. Making copies of immutable objects is simply a waste of effort, so when you try to get a direct copy of any immutable object, you can just about always expect to get the same object back.
    That probably has something to do with the fact that +stringWithFormat: is a convenience method that uses autorelease in a reference-counted environment. If you want to make sure that your object ivar assignments are handled properly by GC, use @property/@synthesize and dot-syntax. That way you will know that GC has been notified of the assignment, otherwise I would not count on it polling or examining objects to see what pointers not to destroy.

    Or you could just learn how to get by without GC.
     
  22. Catfish_Man
    Expand Collapse
    macrumors 68030

    Catfish_Man

    Joined:
    Sep 13, 2001
    Location:
    Portland, OR
    #22
    The garbage collector inserts a write barrier in every assignment so it can track pointers. It will absolutely examine your objects to see what pointers not to destroy. That's how it works.

    Dot syntax and properties literally have no effect on GC. Dot syntax has no effect on anything. It compiles to the same assembly as non-dot syntax.
     
  23. jared_kipe
    Expand Collapse
    macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #23
    Inside your, supremely named, fun1 NSLog out all the variables and see if it NSLogs anything before it crashes. If so, NSLog each one individually until you see which string is not what you think it is.

    If it can NSLog out those strings (the aptly named str1 and str2), please post the entire code of fun1. Based on the information you provided I cannot see these two objects being actual objects, and my money is on str2 not being what you think it is.
     
  24. Aumnayan
    Expand Collapse
    thread starter macrumors newbie

    Joined:
    Dec 4, 2010
    #24
    Putting NSLogEvents into fun1, is irrelevant. The contents of this function can ba replaced with { int a; a = 1; } and it will still crash at the open bracket, with EXC_BAD_ACCESS.

    I've taken the GC out of the equation, since there seams to be a lot of discussion with regards to this, and added the code necessary to retain/release the various variables. I choose to us the GC as either method doesn't give me control over when things get deallocated, and so I figured I might as well give up control completly and save myself a dozen lines of code or so :rolleyes:.

    Here is the code as it stands now, minus only the pieces of the original c file I'm trying to wrap a GUI around.

    fun1 = process3ArcSecondMapFile

    Come of the formatting might be hosed due to copy/paste
    Code:
    main.m
    
    int main(int argc, char *argv[])
    {
      NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
      return NSApplicationMain(argc,  (const char **) argv);
      [pool drain];
    }
    
    GUI.h
    
    #import <Cocoa/Cocoa.h>
    
    @interface GUI : NSObject {
      NSString        *MapPathString;
      NSString        *OutputPathString;
      float            totalFiles;
      int              currentFile;
      
      IBOutlet NSTextField           *mapPath;
      IBOutlet NSTextField           *fullPath;
      IBOutlet NSTextField           *progressBarLabel;
      IBOutlet NSProgressIndicator   *progressBar;
      IBOutlet NSButton              *ExitButton;
      IBOutlet NSButton              *GenerateButton;
      NSTimer                        *updateTimer;
    }
    
    - (IBAction) StartMapGen:(id)sender;
    - (void) timerFireMethod:(NSTimer*)theTimer;
    
     - (void) processThread;                                 
    - (void) process3ArcSecondMapFile:(NSString*)inFile lat: (int)lat lon:(int)lon outPath:(NSString*) outPath;                                
    @end
    
    
    GUI.m
    
    #import "GUI.h"
    #import <dirent.h>
    
    @implementation GUI
    
    - (IBAction) StartMapGen:(id) sender
    {
      MapPathString = [NSString stringWithString:[[mapPath stringValue] copy]];
      OutputPathString = [NSString stringWithString:[[fullPath stringValue] copy]];
      currentFile = 0;
      totalFiles = 0;
      
      [MapPathString retain];
      [OutputPathString retain];
      
      DIR *pdir;
      struct dirent *entry;
      
      pdir = opendir([MapPathString UTF8String]);
      if (pdir) {
        entry = readdir(pdir);
      
        while (entry) {
          if (strcmp(entry->d_name, ".") && strcmp(entry->d_name, "..")) {
            totalFiles++;
          }
          entry = readdir(pdir);
        }
        
        closedir(pdir);
      
        if (totalFiles > currentFile) {
          [progressBar setMaxValue:totalFiles];
          [progressBar setDoubleValue:currentFile];
          [progressBar startAnimation:self];
          [GenerateButton setEnabled:FALSE];
          [NSThread detachNewThreadSelector:@selector(processThread) 
                                 toTarget:self 
                                 withObject:nil];
        }
      }
    }
    
    - (void) processThread
    {
      NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
      DIR *pdir;
      struct dirent *entry;
      NSString *outPath = [[NSString alloc] initWithString:OutputPathString];
      [outPath retain];
      
      pdir = opendir([MapPathString UTF8String]);
      if (pdir) {
        entry = readdir(pdir);
        while (entry) {
          if (strcmp(entry->d_name, ".") && strcmp(entry->d_name, "..")) {
            if (!strncmp(entry->d_name + (strlen(entry->d_name) - 4), 
                         ".hgt", 4)) {          
              int lat, lon;
              char tmp[4];
              
              tmp[0] = entry->d_name[1]; tmp[1] = entry->d_name[2]; tmp[2] = '\0';
              lat = ((entry->d_name[0] == 'S') ? -1: 1) * atoi(tmp);
              
              tmp[0] = entry->d_name[4]; tmp[1] = entry->d_name[4]; 
              tmp[2] = entry->d_name[6]; tmp[3] = '\0';
              lon = ((entry->d_name[3] == 'W') ? -1: 1) * atoi(tmp);
                        
              lon = (360 + lon) % 360;
              lat = (90 - lat) % 180;
              
              NSString *inFilePath = [[NSString alloc] initWithFormat:@"%@/%s",
                                      MapPathString, entry->d_name];
              [inFilePath retain];
    //At this point all NSString values & ints are what I expect them to be.
              [self process3ArcSecondMapFile: inFilePath lat:lat lon:lon outPath:outPath];
              [inFilePath release];
            }
          }
          currentFile++;
          entry = readdir(pdir);
        }
        closedir(pdir);
      }
      
      [MapPathString release];
      [OutputPathString release];
      [outPath release];
      [pool drain];
    }
    
    - (void) process3ArcSecondMapFile:(NSString*)inFile lat: (int)lat lon:(int)lon outPath:(NSString*) outPath
    {  // Crashes at this LINE with EXC_BAD_ACCESS, inFile and outPath invalid.
      int a = 0;
    	a++;
    
      
    }
    @end
    
    
     
  25. kainjow
    Expand Collapse
    Moderator emeritus

    kainjow

    Joined:
    Jun 15, 2000
    #25
    It looks like you're still missing code there, because I don't see how that crash could occur where you say it occurs.

    Even then, your memory management is wonky and incorrect.

    It'd be helpful if you could post a complete compilable project (not just the code).
     

Share This Page