Barracuda09 / SATPI

SATIP server for linux, suitable to run on an Raspberry Pi, Vu+, GigaBlue or any other linux box. currently supporting DVB-S/S2/T/C and transform DVB-S requests to DVB-C
http://barracuda09.github.io/SATPI
GNU General Public License v2.0
155 stars 32 forks source link

BUG: Child PIPE Error: FATAL: exception not rethrown, and zombie subprocess #95

Closed lars18th closed 4 years ago

lars18th commented 4 years ago

Hi @Barracuda09 ,

I'm testing the "Child PIPE" input and I see this error:

FATAL: exception not rethrown
Aborted

This appears when using something like this: rtsp://%1/?msys=childpipe&exec="wget%20-qO-%20http:%2F%2F192.168.1.30:80%2F%3Fsrc=1%26freq=474%26pol=%26msys=DVBT%26sr=0%26pids=all"

And the "wget" process continues open. I feel you need to add something to force killing child process when an exception appears. The worst is that after this, the satpi process doesn't really close, and I can't start it another time. Only after manually killing the "wget" process I can restart. So I feel, this requires some attention.

Please, can you fix it? Thank you.

lars18th commented 4 years ago

Hi,

The cause of the error is because the wget requires a quote over the URL, so with this it works: rtsp://%1/?msys=childpipe&exec="wget%20-qO-%20'http:%2F%2F192.168.1.30:80%2F%3Fsrc=1%26freq=474%26pol=%26msys=DVBT%26sr=0%26pids=all'"

But without the single quotes over the URL the process fails. So, I feel that you can reproduce the error in house. In that case then it will be easy to debug and fix it. Even the command is invalid, it will be desirable that erroneous commands don't generate such troubles.

Regards.

lars18th commented 4 years ago

Hi @Barracuda09 ,

Another error with "Child pipe":

* Error writing output file

This appears when the SAT>IP client closes the session. And after that, nothing works with this virtual front end. So, I need to restart SATPI.

I suggest to re-init the frontend (the Child Pipe) if you see any unhandled error. I feel this will make this module more robust. And don't forget to kill/close all created subprocess.

Regards.

Barracuda09 commented 4 years ago

Hi @lars18th

https://github.com/Barracuda09/SATPI/blob/f33d58c3ab3f3fc708418047a2b7dae43af8013f/src/base/ThreadBase.cpp#L146-L154

Could you try adding after line 150 a throw; and see if this resolves this issue?

lars18th commented 4 years ago

Hi @Barracuda09 ,

Could you try adding after line 150 a throw; and see if this resolves this issue?

I confirm that this new line throw; will solve the first case: FATAL: exception not rethrown. So when the wget URL doens't have the quotes, then wget%20-qO- http... doesn't blocks the SATPI process (but obviosly doesn't work, as the quotes are required). So, please apply it! :+1:

However, the second case: * Error writing output file seems to not be related to this part. In this case, this message is printed by the child process executed. I feel you need first to kill the subprocess and then close the pipe file. Please, can you check it?

Regards.

lars18th commented 4 years ago

Hi @Barracuda09 ,

Even the throw; solves the fatal error FATAL: exception not rethrown, the wget process continues live in background. So, you need to kill it. After closing the SATPI process, they continue here and the listening socket of SATPI continues open. So to restart I need to first kill the wget.

Please, check to enforce to kill any subprocess! Regards.

lars18th commented 4 years ago

Hi @Barracuda09 ,

If I put a "cat" process at the end of chain in the mapping.m3u like this:

#EXTINF:-1 satip-freq="10744", source
rtsp://%1/?msys=childpipe&exec="/bin/processor%20|%20cat"

Then the error is:

cat: write error: Broken pipe
* Error writing output file

And the SATPI process is stalled (I need to close it with CTRL+C).

So, please check this when you have time. Without a robust subprocess handling it's impossible to use the Child Pipe front end. Regards.

Barracuda09 commented 4 years ago

Hi @lars18th

Yes, I will try to fix this

Barracuda09 commented 4 years ago

Hi @lars18th

What would be an valid URL for this example? rtsp://192.168.1.1/?msys=childpipe&exec="wget -qO- http://192.168.1.30:80/?src=1&freq=474&&msys=DVBT&sr=0&pids=all"

wget need quotes around http request or percent encoded.

lars18th commented 4 years ago

What would be an valid URL for this example? rtsp://192.168.1.1/?msys=childpipe&exec="wget -qO- http://192.168.1.30:80/?src=1&freq=474&&msys=DVBT&sr=0&pids=all"

wget need quotes around http request or percent encoded.

My previous second example includes the correct syntax (with single quotes): https://github.com/Barracuda09/SATPI/issues/95#issuecomment-703089533

So in your exemple, and without the %20 for spaces (percent encoding), it will be:

rtsp://192.168.1.1/?msys=childpipe&exec="wget -qO- 'http://192.168.1.30:80/?src=1&freq=474&&msys=DVBT&sr=0&pids=all'"

I hope that with this example you have all the information to reproduce/debug the problem and fix it. Regards.

Barracuda09 commented 4 years ago

Hi @lars18th

Yes I can reproduce it. The only thing is that I have to parse it correctly to get the exec="...."

So I think it should be more like (I beleave VLC is using percent encoding when using M3U file) rtsp://192.168.1.1/?msys=childpipe&exec=%22wget%20-qO-%20%27http:....

lars18th commented 4 years ago

Hi @Barracuda09 ,

Yes, the percent encoding is a requirement for URI links, so then you need to use the %27 for the single quotes too, like in your example: rtsp://192.168.1.1/?msys=childpipe&exec=%22wget%20-qO-%20%27http:....

So, please include in the examples the correspondence without the percent encoding inside a comment. And futhermore, if you print the target URL in the Log/UI remember to print the unencoded version, as it will be more user-friendly.

Thank you!

Barracuda09 commented 4 years ago

Hi @lars18th

I have made a fix for this in branch V1.6.2, Please check it out.

lars18th commented 4 years ago

Hi @Barracuda09

I have made a fix for this in branch V1.6.2, Please check it out.

After some testing, this seems to be fixed. So, from my point of view you can merge it.

However, one advise: If you use an incorrect PIPE command in the translation it's quite difficult to debug it. For example, when using "wget -qO- http..." you don't need to use any quotes internally, only spaces (escaped). Perhaps, some testing can be provided: for example, one textbox and button in the UI to execute the command and check if it runs. Then you can print the escaped version to copy it to the m3u file. That's only a suggestion/idea.

Thank you!

Barracuda09 commented 4 years ago

Reopen

lars18th commented 4 years ago

Hi @Barracuda09 ,

Regarding the quotes of the commands, I finally started to use SCRIPTS. Then it will be more easy, as no escaped versions are required. One example:

File: script-10744.sh

#!/bin/sh
stream-binary --parameter 10744

File: script-10730.sh

#!/bin/sh
wget -qO- http://server/?iptv=

And then inside the mapping.m3u:

#EXTINF:-1 satip-freq="10744", 
rtsp://%1/?msys=childpipe&exec="/opt/satpi/script-10744.sh"
#EXTINF:-1 satip-freq="10730", 
rtsp://%1/?msys=childpipe&exec="/opt/satpi/script-10730.sh"

I comment this here because perhaps you want to include this info inside the examples. Regards.

lars18th commented 4 years ago

Hi @Barracuda09 ,

I suggest to close this issue if you aprove to include this example: https://github.com/Barracuda09/SATPI/issues/102#issuecomment-729649616

So, from my point of view this is complete. Regards.

Barracuda09 commented 4 years ago

Hi @lars18th

Yes I will try to implement you suggestions as soon as possible.

Barracuda09 commented 4 years ago

Will implement #102 and discus further there.

lars18th commented 4 years ago

Will implement #102 and discus further there.

OK, we'll continue the discussion in #102 .