Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String.hashCode returns different values with same sequence #20122

Closed
najeira opened this issue Aug 2, 2018 · 13 comments
Closed

String.hashCode returns different values with same sequence #20122

najeira opened this issue Aug 2, 2018 · 13 comments
Assignees
Labels
dependency: dart Dart team may need to help us

Comments

@najeira
Copy link
Contributor

najeira commented Aug 2, 2018

Steps to Reproduce

String.hashCode returns different values with same sequence.

This only happens when the app is running on iOS simulator/device.

import 'package:flutter/material.dart';

void main() {
  runApp(new MaterialApp(
    home: new MyApp(),
  ));
}

class MyApp extends StatefulWidget {
  @override
  State<StatefulWidget> createState() {
    return new MyAppState();
  }
}

class MyAppState extends State<MyApp> {
  final Map<Object, dynamic> map = <Object, dynamic>{};
  
  Foo foo1;
  Foo foo2;
  
  @override
  void initState() {
    super.initState();
    
    foo1 = _foo("test");
    foo2 = _foo("test");
    
    // this is that!
    map[foo1] = foo1;
  }
  
  @override
  Widget build(BuildContext context) {
    return new Scaffold(
      appBar: new AppBar(
        title: new Text("String.hashCode"),
      ),
      body: new Container(
        padding: const EdgeInsets.all(20.0),
        child: new Column(
          children: <Widget>[
            new Text("${foo1.name} ${foo1.name.hashCode}"),
            new Text("${foo2.name} ${foo2.name.hashCode}"),
          ],
        ),
      ),
    );
  }
}

Foo _foo(String name) {
  return new Foo("prefix-${name}", "value");
}

class Foo {
  const Foo(this.name, this.value);
  
  final String name;
  final String value;
  
  @override
  int get hashCode {
    return identityHashCode(name) ^ identityHashCode(value);
  }
  
  @override
  bool operator ==(other) {
    if (identical(this, other)) {
      return true;
    }
    return other is Foo 
      && this.name == other.name 
      && this.value == other.value;
  }
}

Therefore, String can not be used as the key of Map.

Logs

[✓] Flutter (Channel beta, v0.5.1, on Mac OS X 10.12.6 16G1314, locale ja-JP)
    • Flutter version 0.5.1 at /Applications/flutter
    • Framework revision c7ea3ca377 (9 weeks ago), 2018-05-29 21:07:33 +0200
    • Engine revision 1ed25ca7b7
    • Dart version 2.0.0-dev.58.0.flutter-f981f09760

[✓] Android toolchain - develop for Android devices (Android SDK 27.0.3)
    • Android SDK at /Users/najeira/Library/Android/sdk
    • Android NDK at /Users/najeira/Library/Android/sdk/ndk-bundle
    • Platform android-27, build-tools 27.0.3
    • ANDROID_HOME = /Users/najeira/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1024-b01)
    • All Android licenses accepted.

[✓] iOS toolchain - develop for iOS devices (Xcode 9.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 9.2, Build version 9C40b
    • ios-deploy 1.9.2
    • CocoaPods version 1.5.2

[✓] Android Studio (version 3.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    ✗ Flutter plugin not installed; this adds Flutter specific functionality.
    • Dart plugin version 171.4424
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1024-b01)

[✓] IntelliJ IDEA Ultimate Edition (version 2018.1)
    • IntelliJ at /Applications/IntelliJ IDEA.app
    • Flutter plugin version 23.1.3
    • Dart plugin version 181.4203.498

[!] VS Code (version 1.25.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension not installed; install from
      https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter

[✓] Connected devices (1 available)
    • iPhone X • 3264D9F7-7AB5-4C13-8863-85CEC0730DB1 • ios • iOS 11.2 (simulator)

I tested it on v0.4.4, v0.5.1, v0.5.7.

@mraleph mraleph added the dependency: dart Dart team may need to help us label Aug 2, 2018
@mraleph mraleph self-assigned this Aug 2, 2018
@mraleph
Copy link
Member

mraleph commented Aug 2, 2018

str1 and str2 has same sequence, but different hashCodes:

Is the output you provided hypothetical or it is from the real application - meaning that in your application string abc has different hash codes? The reason I am asking - it would help to know which string exactly shows the behavior.

@najeira
Copy link
Contributor Author

najeira commented Aug 2, 2018

Thank you very much for replying.

The actual output is this:

flutter: 'hoge-test' 849354836
flutter: 'hoge-test' 455516103
  • someFunc makes a string like return "prefix-${value}".
  • It occurs in debug and release on iOS. not occurs on Android.
  • It occurs with both Latin and Japanese strings.

I found that when someFunc is changed to return value from return "prefix-${value}", it does not occur.

@mraleph
Copy link
Member

mraleph commented Aug 2, 2018

@najeira just to clarify: does you function actually say return "hoge-${value}" (based on the output)?

@najeira
Copy link
Contributor Author

najeira commented Aug 2, 2018

@mraleph No, it's more complicated.

My code like this:

class Foo {
  Foo(this.name);
  final String name;
}

Foo someFunc(String name) {
  return new Foo("prefix-${name}");
}
var foo1 = someFunc("test");

// ...

var foo2 = someFunc("test");

// ...

debugPrint(foo1.name == foo2.name); // true
debugPrint(foo1.name.hashCode == foo2.name.hashCode); // false

@najeira
Copy link
Contributor Author

najeira commented Aug 2, 2018

If debugPrint is inserted after someFunc, there is no problem.

var foo1 = someFunc("test");
debugPrint("${foo1.name} ${foo1.name.hashCode}"); // prefix-test 795436880

// ...

var foo2 = someFunc("test");
debugPrint("${foo2.name} ${foo2.name.hashCode}"); // prefix-test 795436880

// ...

debugPrint(foo1.name == foo2.name); // true
debugPrint(foo1.name.hashCode == foo2.name.hashCode); // true

@mraleph
Copy link
Member

mraleph commented Aug 2, 2018

I think we might need a reproduction of this to actually figure it out - I reviewed logic for hash computation and it does not seem like there are any bugs there.

I have tried to trigger various misoptimizations - but so far I was not successful.

@najeira
Copy link
Contributor Author

najeira commented Aug 2, 2018

@mraleph I found the reproduction code, I updated the first comment.

The followings are important for this:

  • Foo overrides hashCode and use identityHashCode.
  • Inserting Foo object to Map as key.

@mraleph
Copy link
Member

mraleph commented Aug 2, 2018

Thanks for the additional information! Reduction:

make(v) => 'x${v}';

void main() {
  final x = make('test');
  final y = make('test');
  identityHashCode(y);  // initialize hidden hash field with identity hash.
  print(x.hashCode == y.hashCode);  // will print false on x64 and ARM64.
}

The issue is that identityHashCode should treat strings and some other types specially when we cache object hash in the object header.

I will submit a fix upstream.

@mraleph
Copy link
Member

mraleph commented Aug 2, 2018

The fix is uploaded upstream: https://dart-review.googlesource.com/c/sdk/+/68042

@zoechi zoechi added the waiting for PR to land (fixed) A fix is in flight label Aug 2, 2018
@najeira
Copy link
Contributor Author

najeira commented Aug 2, 2018

I will update my app so that I do not use identityHashCode:

@override
int get hashCode {
  return name.hashCode ^ value.hashCode;
}

Thank you!

dart-bot pushed a commit to dart-lang/sdk that referenced this issue Aug 2, 2018
…shCode

On 64-bit platforms we use a field in the header to cache results of
Object.get:_identityHashCode. The very same field is also used to
cache String.get:hashCode result. Which means that their implementations
must be the same.

Fixes flutter/flutter#20122

Change-Id: I98eef9eddf833c0d7c4c6f452728fe48e232efdc
Reviewed-on: https://dart-review.googlesource.com/68042
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
@mraleph
Copy link
Member

mraleph commented Aug 2, 2018

The fix has landed upstream. Waiting for the roll into Flutter to pick it up.

@mraleph
Copy link
Member

mraleph commented Aug 20, 2018

Dart with a fix has reached Flutter. Closing.

@mraleph mraleph closed this as completed Aug 20, 2018
@mraleph mraleph removed the waiting for PR to land (fixed) A fix is in flight label Aug 20, 2018
@github-actions
Copy link

github-actions bot commented Sep 2, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependency: dart Dart team may need to help us
Projects
None yet
Development

No branches or pull requests

3 participants