Crash regression caused by replacing NSString.hash with java.lang.String hash

[brunobowden]

This regressed in commit @421bc62383895eae80b813c841f83e58e625158b which replaced NSString.hash with java.lang.String hash. This affects the iOS code that handles access to managed objects, causing crashes in certain circumstances. It was originally discovered thanks to @harrycheung.

Test Case (just change the J2OBJC_HOME setting within Xcode): https://github.com/harrycheung/testscroll

Thread Discussion: https://groups.google.com/forum/#!topic/j2objc-discuss/CYpop1IyRm8

Based on @harrycheung's test case, I noticed it regressed between j2objc 0.9.3 and 0.9.4, then did a binary search within those changes. Oscillating around commits 421bc62 and 641f462 will fix the crash. Reverting 421bc62 on HEAD did not fix it, so it's not a clear solution.

The crash occurs during a call to NSFetchedResultsController. Producing the error that Harry shared before:

2014-12-11 22:00:45.904 TestScroll[39713:3263240] *\ Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'keypath timeStamp not found in entity '

What's weird is that it only happens if there are zero managed objects. If a single managed object exists then no crash occurs. I suspect that searching managed objects use hashing and maybe the jre_emul hash code? Perhaps a pre-existing object has a differently calculated hash averts the crash? I don't have a good guess on this.

There's some other unusual properties about... Harry noticed that if you comment out the Math.cos(40) line then it works without a crash. Curiously, you don't even need to call the Math.cos(40) method, you only need to link it! If I comment out the instantiation of the Car object in Swift (but still translate the code), then the crash still occurs.

If I change the code to the following, then commenting out the single assignment of Long.MAX_VALUE will fix the crash. Without that line, all you have is a Long* pointer and my guess is that the linker doesn't have to link in the active code.

package harrycheung;

import java.lang.Long;

public final class Car {
    public Car() {
        Long lng;
        // Comment out following line to avoid crash
        lng = Long.MAX_VALUE;
    }
}

[brunobowden]

Reverting 421bc62 on HEAD - DOES fix it. This is a correction to what I said before.

This includes changes up to 8055e442da4d4954f2007e6b469804b88c0c69d8.

[brunobowden]

@tomball @kstanger @harrycheung

Would it be possible / sensible to fix the hashing code in the following manner?

1) NSString hash() left unmodified 2) NSString javahash() added as a new method 3) All translated code calls javahash() instead of hash() 4) Instruct developers to be aware of the differences between hash and javahash for custom code written in Objective-C or Swift... but most shouldn't encounter this as an issue

I don't know enough about j2objc to understand all the implications. Effectively you're creating two parallel hashing systems that produce different results and don't interact with each other. This may mean an extra step in translation as you're changing the "hash" method name to avoid a collision within the iOS namespace. For consistency, it's perhaps best to change hash to javahash throughout all the other objects as well - even though only NSString causes an issue?

The goal is to address the following conflicting priorities:

A) Maintain the behaviour of iOS hashing to avoid crashes and other unknown issues B) Maintain the behaviour of Java hashing to keep consistent behaviour and easier client <> server communication if you use hashing for that C) Make this transparent to developers as much as possible

Drop me a note when you publish the changes for Jre_Emulation.xcodeproj and I'll delve in a little further as to how iOS is using hash for managed objects. That may provide an alternate solution but I'm skeptical that changing iOS hashing could cause other unexpected issues.

[brunobowden]

From my reading of the iOS documentation, the behaviour is undefined if you change an existing method. I don't have enough experience to know the practical implications...

https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/CustomizingExistingClasses/CustomizingExistingClasses.html#//apple_ref/doc/uid/TP40011210-CH6-SW4

If the name of a method declared in a category is the same as a method in the original class, or a method in another category on the same class (or even a superclass), the behavior is undefined as to which method implementation is used at runtime. This is less likely to be an issue if you’re using categories with your own classes, but can cause problems when using categories to add methods to standard Cocoa or Cocoa Touch classes.

[tomball]

Should be fixed in latest source, as the overriding of NSString hash has been rolled back.