Rust 1.20 regression, rstored

[brson]

https://github.com/artur-augustyniak/rstored

brian@ip-10-145-43-250:/mnt2/dev/rstored⟫ git log -1
commit c0032b5061cbacd2c5fec894b8b4dc718a2975c3
Author: Artur Augustyniak <artur@aaugustyniak.pl>
Date:   Sat May 13 13:39:25 2017 +0200

    todo up

brian@ip-10-145-43-250:/mnt2/dev/euclid⟫ rustc +nightly -Vv
rustc 1.20.0-nightly (622e7e648 2017-06-21)
binary: rustc
commit-hash: 622e7e6487b6fb7fdbb901720cd4214f9179ed67
commit-date: 2017-06-21
host: x86_64-unknown-linux-gnu
release: 1.20.0-nightly
LLVM version: 4.0

brian@ip-10-145-43-250:/mnt2/dev/rstored⟫ cargo +nightly test
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running target/debug/deps/rstored-c51de7b5c16d0250

running 2 tests
test logging::logger::tests::test_stdout_match ... ok
test logging::logger::tests::test_syslog_match ... FAILED

failures:

---- logging::logger::tests::test_syslog_match stdout ----
        syslog_log(1, foo)
thread 'logging::logger::tests::test_syslog_match' panicked at 'assertion failed: `(left == right)` (left: `2`, right: `1`)', src/logging/logger.rs:141
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    logging::logger::tests::test_syslog_match

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--bin rstored'

cc @artur-augustyniak

[Mark-Simulacrum]

As best as I can tell, this is broken code. I don't really know what caused it to fail, but it appears to be transmuting a non-repr(C) enum and expecting a given result. In fact, I believe it's thread-unsafe -- it's mutating the static mut's here https://github.com/artur-augustyniak/rstored/blob/master/src/logging/logger.rs#L97 without synchronization.

I'm not really following the exact details of it's test though so hard to say for certain.

[artur-augustyniak]

Hi, first of all this test is a real mess. While learning Rust it was an attempt to simulate C derived testing technique, where you replace some non testable code using preprocessor include directive.
I did not know that test functions are run in parallel by default (found RUST_TEST_THREADS=1 right now).
But for clarity, transmuting data was an attempt to get enum ordinal and then save this ordinal in static mut (to check exact option used as parameter via its side effect):

enum Foo{
   Bar,
   Baz
}


use std::mem;

fn main() {
    unsafe{
        let ordinal1: u8 = mem::transmute(Foo::Bar);
        let ordinal2: u8 = mem::transmute(Foo::Baz);
        println!("{} {}", ordinal1, ordinal2);
    }
}

[artur-augustyniak]

I think that this issue is caused only by broken code like @Mark-Simulacrum wrote.
It seems that in case of my code it has nothing to do with enum transmuting.
I tried to run this test using nightly and stable (x86_64 boxes).
Same effect, basic concurrency bug.
Then i wrote snippet below:

use std::thread;
use std::sync::{Arc, Mutex};

static NUM_THREADS: u8 = 4;
static NUM_REP: u32 = 100000;
pub static mut PROG_SUM: u8 = 0;

fn unsafe_sum() {
    let mut thread_handles = vec![];

    for _ in 0..NUM_THREADS {
        thread_handles.push(thread::spawn(move || {
            for i in 1..11 {
                unsafe {
                    PROG_SUM += i;
                }
            }
        }));
    }

    for handle in thread_handles {
        let _ = handle.join();
    }
    let fin_prog_sum;
    unsafe {
        fin_prog_sum = PROG_SUM;
    }
    assert_eq! (NUM_THREADS * 55, fin_prog_sum);
    unsafe {
        PROG_SUM = 0;
    }
}


fn safe_sum() {
    let sum = Arc::new(Mutex::new(0u8));
    let mut thread_handles = vec![];
    for _ in 0..NUM_THREADS {
        let sum_thread_handle = sum.clone();
        thread_handles.push(thread::spawn(move || {
            for i in 1..11 {
                let mut data = sum_thread_handle.lock().unwrap();
                *data += i;
            }
        }));
    }
    for handle in thread_handles {
        let _ = handle.join();
    }
    let final_sum = *sum.lock().unwrap();
    assert_eq! (NUM_THREADS * 55, final_sum);
}

fn main() {
    for _ in 0..NUM_REP {
        safe_sum();
    }
    println!("safe sum done");
    for _ in 0..NUM_REP {
        unsafe_sum();
    }
    println!("unsafe sum done");
}

Despite of rust channel, unsafe function panics from time to time, safe one did not even once.
I thought u8 assignment will be atomic.

[brson]

Thanks for investigating @artur-augustyniak ! Closing per your comments.