reimplement how stdin is read on the cli

[jwestfall69]

This PR re-implements how stdin reading is done on the cli and has the following fixes over the old

1. Doesn't trigger an assert() if running a debug build and in daemon mode
2. Doesn't mess up the terminal if in daemon mode
3. Doesn't listen for stdin input in daemon mode

Fixes #200

[jwestfall69]

Note that I'm limited in how much testing I can do as I'm not setup to build on windows/osx.

[p12tic]

Could we just get rid of listening to stdin? The previous setup of waiting for some symbol is rather uncommon approach and I wonder if that feature is actually used by anyone.

[p12tic]

I think I misread your initial proposal of removing listening to stdin in daemon mode in #200 (comment) as removing listening to stdin altogether. Sorry for the wasted time :-(

[AdrianKoshka]

So should I merge this PR?

[p12tic]

No, let's see what @jwestfall69 replies and also we need testing.

[jwestfall69]

I was originally hoping to just nuke the stdin reading, but when going through the processing of removing the code I found the GUI was relying on it. In the GUI when you 'start' its spawning (QProcess) barrierc or barriers as a sub process. When the user wants to stop in the GUI, the GUI sends the 'S' char to the stdin of the spawned process to signal to it to shutdown/exit.

barrier/src/gui/src/MainWindow.cpp

Lines 727 to 732 in 4dedd88

	if (barrierProcess()->isOpen()) {
	// try to shutdown child gracefully
	barrierProcess()->write(&ShutdownCh, 1);
	barrierProcess()->waitForFinished(5000);
	barrierProcess()->close();
	}

Suppose we could try ditching the stdin bits and just trying to close() the sub process, which the qt docs say will kill it.

[p12tic]

Okay, keeping stdin processing makes sense then, for now :-)

The PR looks good to me, just needs testing on windows and osx.

[jwestfall69]

I would note that the shutdown process (unrelated to this PR) on OSX results in a segfault, which will make it impossible to fully test this PR. It happens on both client/server cli binaries and can be seen if you view the logs while hitting the stop button in the GUI

[2019-01-05T13:41:24] INFO: stopping barrier desktop process
[2019-01-05T13:41:26] ERROR: process exited with error code: 11

[evantandersen]

Hey, can we get this merged? I'm stuck between a rock and hard place right now. I can't run the barrier GUI since flatpak doesn't support mdns and I can't run the client as a service because of this bug. I've temporarily fixed it by having it launch in a gui gnome-terminal session on startup, but I'd like to switch it over to a systemd service.

I have windows, linux and osx vms that I could do some basic tests with need be.

[AdrianKoshka]

I would note that the shutdown process (unrelated to this PR) on OSX results in a segfault, which will make it impossible to fully test this PR.

I'd say this couldn't be merged because of this segfault issue.

[AdrianKoshka]

Going to close this PR, if anyone wants to continue the work done here but figure out the segfaulting issue, feel free. I'd suggest opening a new PR though to run against our new CI.

[jwestfall69]

Little confused. You are closing the PR because of a segfault in barrier on OSX unrelated to the PR that makes it impossible to test the PR on OSX?

[AdrianKoshka]

I closed this PR mostly due to inactivity. I also will add, running against only the old CI doesn't make much sense. If we re-opened this as a new PR, it would be ran against the new CI. We would obviously reference this old closed PR in the new one so the convo isn't lost.

[shymega]

@jwestfall69 /others who run Barrier on OSX who has this segfault issue: if you get a chance to get a core dump when it segfaults, please do -- that'd be superb for debugging.

[jwestfall69]

I created an issue with some details #389

[AdrianKoshka]

new PR is #390