Crash on mismatched property types #103

Closed
beelsebob opened this Issue May 8, 2011 · 4 comments

Comments

Projects
None yet
2 participants
@beelsebob
Contributor

beelsebob commented May 8, 2011

If a property is declared in a header to have a different type to in the implementation appledoc will crash.

Steps To Reproduce:

  1. Create A.h containing:

import <Foundation/Foundation.h>

@interface A : NSObject
@property (readonly,retain) NSString *b;
@EnD
2. Create A.m containing:
@interface A ()
@property (readwrite,retain) NSArray *b;
@EnD
@implementation A
@synthesize b;
@EnD
3. Run the files through appledoc

Expected results:
Documentation is generated – perhaps a warning is thrown.

Actual results:
appledoc crashes:

2011-05-08 15:44:36.322 appledoc[60513:903] *** Assertion failure in -[GBMethodData validateMergeWith:], /Users/tatd2/Documents/appledoc/Model/GBMethodData.m:262
Oops, something went wrong...
NSInternalInconsistencyException: Invalid parameter not satisfying: [source.methodResultTypes isEqualToArray:self.methodResultTypes]
@ 0 CoreFoundation 0x00007fff800b47b4 __exceptionPreprocess + 180
@ 1 libobjc.A.dylib 0x00007fff890ed0f3 objc_exception_throw + 45
@ 2 CoreFoundation 0x00007fff800b45d7 +[NSException raise:format:arguments:] + 103
@ 3 Foundation 0x00007fff86513776 -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 198
@ 4 appledoc 0x0000000100023389 -[GBMethodData validateMergeWith:] + 1225
@ 5 appledoc 0x0000000100023497 -[GBMethodData mergeDataFromObject:] + 247
@ 6 appledoc 0x000000010001ef49 -[GBMethodsProvider registerMethod:] + 585
@ 7 appledoc 0x000000010004abe3 -[GBProcessor mergeKnownCategoriesFromStore] + 3299
@ 8 appledoc 0x0000000100045f4f -[GBProcessor processObjectsFromStore:] + 399
@ 9 appledoc 0x0000000100004aa9 -[GBAppledocApplication application:runWithArguments:] + 1753
@ 10 appledoc 0x00000001000023d3 -[DDCliApplication runWithClass:] + 323
@ 11 appledoc 0x0000000100002738 DDCliAppRunWithClass + 120
@ 12 appledoc 0x00000001000019a7 main + 71
@ 13 appledoc 0x0000000100001954 start + 52

@tomaz

This comment has been minimized.

Show comment Hide comment
@tomaz

tomaz May 9, 2011

Owner

Exactly - appledoc detects different type, which I don't allow, logs it (NSInternalInconsistencyException: Invalid parameter not satisfying: [source.methodResultTypes isEqualToArray:self.methodResultTypes]) and quits. So actually it's not a crash, although the log looks like it - appledoc uses exceptions for quitting; it's simple and doesn't require checking method results all the time. As exceptions include stack trace, I write it out to get more info, because sometimes they really are result of some internal issue. From user point of view though, it would be better to log "known" exits differently to differentiate between them.

Back to your issue: although objective C seems to allow this kind of stuff, I never used it so wasn't aware of it. This could easily be modified though, probably the best option would be to use interface declaration. I can see usefulness of this approach, for example for hiding internal types from public interfaces - id in interface and actual type in implementation, although it doesn't make sense using unrelated types as in your example ;)

Owner

tomaz commented May 9, 2011

Exactly - appledoc detects different type, which I don't allow, logs it (NSInternalInconsistencyException: Invalid parameter not satisfying: [source.methodResultTypes isEqualToArray:self.methodResultTypes]) and quits. So actually it's not a crash, although the log looks like it - appledoc uses exceptions for quitting; it's simple and doesn't require checking method results all the time. As exceptions include stack trace, I write it out to get more info, because sometimes they really are result of some internal issue. From user point of view though, it would be better to log "known" exits differently to differentiate between them.

Back to your issue: although objective C seems to allow this kind of stuff, I never used it so wasn't aware of it. This could easily be modified though, probably the best option would be to use interface declaration. I can see usefulness of this approach, for example for hiding internal types from public interfaces - id in interface and actual type in implementation, although it doesn't make sense using unrelated types as in your example ;)

@ghost ghost assigned tomaz May 9, 2011

@beelsebob

This comment has been minimized.

Show comment Hide comment
@beelsebob

beelsebob May 9, 2011

Contributor

Agreed, the example given here doesn't make sense. I should probably file a separate bug, but making exceptions the normal exit for failure seems like a big bug to me. From a correctness point of view, UNIX apps are simply meant to exit with a non-0 return... From a user point of view though, they see a core dump from your code rather than a useful error. "NSInternalInconsistencyException" indicates "I'm in a state the developer thought I should never be in", and the error associated with it does not explain at all why or where it failed – I had to run appledoc in the debugger to figure out why it was crashing.

I'd suggest a top level @Try{ } @catch { } structure in main() to catch these exceptions and print more user friendly output – probably in a log format that things like Xcode can understand (as you do for warnings). I'd completely understand if it were #ifdef DEBUGed around though.

Contributor

beelsebob commented May 9, 2011

Agreed, the example given here doesn't make sense. I should probably file a separate bug, but making exceptions the normal exit for failure seems like a big bug to me. From a correctness point of view, UNIX apps are simply meant to exit with a non-0 return... From a user point of view though, they see a core dump from your code rather than a useful error. "NSInternalInconsistencyException" indicates "I'm in a state the developer thought I should never be in", and the error associated with it does not explain at all why or where it failed – I had to run appledoc in the debugger to figure out why it was crashing.

I'd suggest a top level @Try{ } @catch { } structure in main() to catch these exceptions and print more user friendly output – probably in a log format that things like Xcode can understand (as you do for warnings). I'd completely understand if it were #ifdef DEBUGed around though.

@beelsebob beelsebob closed this May 9, 2011

@beelsebob beelsebob reopened this May 9, 2011

@beelsebob

This comment has been minimized.

Show comment Hide comment
@beelsebob

beelsebob May 9, 2011

Contributor

Sorry, clicked the wrong button there.

Contributor

beelsebob commented May 9, 2011

Sorry, clicked the wrong button there.

@tomaz

This comment has been minimized.

Show comment Hide comment
@tomaz

tomaz May 9, 2011

Owner

Agree - although in this case the code IS in a state "developer" thought it shouldn't be :))) As I'm using NSParameterAssert for catching these, it results in NSInternalInconsistencyException. Which is fine, so we can catch them and properly implement the stuff.

Owner

tomaz commented May 9, 2011

Agree - although in this case the code IS in a state "developer" thought it shouldn't be :))) As I'm using NSParameterAssert for catching these, it results in NSInternalInconsistencyException. Which is fine, so we can catch them and properly implement the stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment