HashMap enters unreachable code in try_insert

[twissel]

Code to reproduce:

use flurry::HashMap;
use rand::{thread_rng, Rng};
use flurry::epoch::pin;

fn main() {
    let mut rng = thread_rng();
    let map = HashMap::new();
    let g = pin();
    for _ in 0..10000 {
        let el = rng.gen_range(0, 1000);
        let _ = map.try_insert(el, el, &g);
    }
}

Output:

thread 'main' panicked at 'internal error: entered unreachable code: no_replacement cannot result in PutResult::Replaced', <::std::macros::panic macros>:5:6
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:77
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1052
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1426
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:204
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:224
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:472
  11: rust_begin_unwind
             at src/libstd/panicking.rs:380
  12: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:334
  13: flurry::map::HashMap<K,V,S>::try_insert
             at ./<::std::macros::panic macros>:5
  14: flurry_test::main
             at src/main.rs:11
  15: std::rt::lang_start::{{closure}}
             at /rustc/b8cedc00407a4c56a3bda1ed605c6fc166655447/src/libstd/rt.rs:67
  16: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  17: std::panicking::try::do_call
             at src/libstd/panicking.rs:305
  18: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:86
  19: std::panicking::try
             at src/libstd/panicking.rs:281
  20: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  21: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  22: std::rt::lang_start
             at /rustc/b8cedc00407a4c56a3bda1ed605c6fc166655447/src/libstd/rt.rs:67
  23: main
  24: __libc_start_main
  25: _start
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Process finished with exit code 101

[jonhoo]

Oh, that's very interesting. This looks like it triggers one of the assertions that @domenicquirl added a while back — they may have a better idea of what triggers this than I do. I don't think this is a result of try_insert specifically (from #74 by @GarkGarcia).

[domenicquirl]

I had to look around a bit, the problem doesn't actually have to do with tree bins. It's also not with try_insert, since the assumption should be correct. Was reproducible (Win) though.

I believe we were missing an early return for when we stop insertion on the slow path. Before we added the Result to put, the value trying to be inserted would simply disappear. I moved this memory cleanup to happen explicitly as part of "don't allocate a node when we don't need one" which snuck in with the tree bin changes, but it's possible that handling this case simply got lost in the merge.

Anyways, I can no longer reproduce the issue after #91. Codecov is complaining about the patch, but otherwise that should fix it.