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

Print immediately from a LogSender if read_line() call is not in progress #30

Closed
remexre opened this issue Mar 1, 2018 · 6 comments
Closed

Comments

@remexre
Copy link
Contributor

remexre commented Mar 1, 2018

This is again because I'm making a log logger that outputs without messing up linefeed, and would also be a breaking change.

Since LogSender writes only actually get written during read_line, something like the following has unintuitive (albeit documented) behavior:

fn main() {
    let mut reader = Reader::new("...").unwrap();
    logger::init(reader.get_log_sender()).unwrap();
    loop {
        let n: usize = match reader.read_line().unwrap() {
            ReadResult::Input(s) => s.parse().unwrap(),
            _ => unimplemented!(),
        };
        
        let out = (0..n).into_par_iter()
            .map(process)
            .sum();
        println!("Got {}", out);
    }
}

fn process(x: usize) -> usize {
    info!("About to process {}", x);
    // pretend this is work-intensive enough that rayon actually parallelizes
    // it, so we actually need the LogSender
    x + 1
}

One would expect that the output would look like:

About to process 0
About to process 1
About to process 2
About to process 3
Got 10

But instead, you get:

Got 10
About to process 0
About to process 1
About to process 2
About to process 3

since the LogSender-sent messages aren't printed until the next call to read_line.

Something (that I now realize won't work) would be to change the _send_counter in LogSender to be Arc<Mutex<()>>, and lock it whenever in read_line or printing directly from LogSender? Then if LogSender can't lock it, it'll send the messages via the channel.

There's a data race there, though, if Reader has the lock, LogSender checks for the lock and fails to acquire it, and Reader drops it and exits before LogSender finishes sending the message on the Sender<String>. If you can think of a way to implement this that doesn't have the race, I'd be happy to write the actual code.

@survived
Copy link

Do you want to have a global output point in LogSender? To my mind it isn't its responsibility. LogSender is primitive of Reader, so it doesn't have to provide all output. It only have a knowledge about how to print on terminal without corrupting a prompt.

@remexre
Copy link
Contributor Author

remexre commented Apr 21, 2018

Is there a non-racy way to solve this use-case without cooperation from LogSender?

@murarth
Copy link
Owner

murarth commented Apr 21, 2018

I am currently working on a refactor that would make Reader usable simultaneously from multiple threads, using internal locking. This would obsolete the LogSender API (which could be replaced with a simple write_line method, which would internally check whether read_line is in progress and behave accordingly) and solve some other issues I have with the internal implementation (e.g. it's gross and I hate it).

@survived
Copy link

You definitely have to use LogSender. I'd write a primitive Switcher (to escape modifying the lib) that has a state indicating is read_line active and a method write_fmt which writes in LogSender or stdout (depends on a state of switcher). But I now realize it won't work :)

When read_line is done and Switcher is waiting for locking itself to change the state, messages are able to slip and spoil the sequence.

I believe the best way to solve the problem is making LogSender throwaway. I mean read_line must make sure that log_channel doesn't has any messages before exiting of itself and then self.log_channel = None to make all LogSenders unfit for use. In that case in theory Switcher are able to switching between LogSender (we have to refresh it after each read_line) and stdout without any troubles.

And probably we should use sync_channel because unlike regular channel it guarantees receiving message or failing on send.

@survived
Copy link

Yea, a simple write_line would be much better.

@murarth
Copy link
Owner

murarth commented May 20, 2018

The changes described in my previous comment have now been implemented: The LogSender API has been removed. Writing to the terminal concurrent with a read_line call can be done without channels, using the Writer type.

@murarth murarth closed this as completed May 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants