karip / harbour-file-browser

A native file browser to view files on Sailfish phones.
The Unlicense
21 stars 19 forks source link

A process will now wait until the previous process is done before starting #10

Closed benna86 closed 9 years ago

benna86 commented 9 years ago

As it was before it would cause a race condition if the second process is spawned before the first one is finished. It might never happen in your code but I think it is better not to have race conditions laying around.

I hope you find my contribution useful, cheers!

karip commented 9 years ago

In the old code, if the second process is spawned before first one is finished, won't it just run two processes at the same time? The only problem I can see is that it will read the output of both processes and combine them into one string array. Can you be more specific and explain where the race condition may happen?

Thanks for going through the code, it's good that it gets reviewed. :)

benna86 commented 9 years ago

You are welcome! Yes, I ment a race condition in the output. I know mine is just a workaround and having separated outputs would be better, but I cannot find a nicer way to do this than subclassing QProcess and give it a private output, but this will require many changes in the code. Do you have a better idea?

karip commented 9 years ago

I wouldn't call the old code a race condition. It is just a bit weird behavior. :) In fact, you can do the same on a Linux command shell. Type these two commands and you'll see similar interleaved output:

> ping -i 5 github.com &
> ping -i 5 yahoo.com

I agree that it is a bit weird behavior. However, File Browser shouldn't have a situation where this happens.

But I can fix it if you really want so. I'll also change the function to return true if a new process was launched, because it may happen that it doesn't get launched.