242 replies to this topic

[WanWizard]

But nobody seems to know why? So chances are the root cause isn't fixed, just covered up?

[Huevos]

But nobody seems to know why? So chances are the root cause isn't fixed, just covered up?

Simon knew, but he is not around anymore.

[littlesat]

…

Edited by littlesat, 20 August 2023 - 21:09.

[Huevos]

But nobody seems to know why? So chances are the root cause isn't fixed, just covered up?

So just for your information the problem with Broadcom and efilepush started here: https://github.com/O... 6575c1d5a985d9 which was useful at least in highlighting other problems with the record threading.

 

Simon confirmed that the recording (and playback) signalling setup didn't work, which leaves the recording thread vulnerable to deadlocks should it ever empty the read buffer. Maybe it worked 5 years ago, but running some tests where he disabled the DMX input into the file used as the source, he observed ::read in eFilePushThreadRecorder::thread() blocking pending some data to read and not responding to SIGUSR1 raised by the main thread.

PR to fix this here: https://github.com/O...nigma2/pull/593

 

If problems like this take 3 years to get looked at properly it is not surprising people that already fixed them elsewhere don't remember the minutiae.

Edited by Huevos, 20 August 2023 - 21:23.

[littlesat]

So you're saying this "PR to fix this here: https://github.com/O...nigma2/pull/593" is the medicine?

 

If so then many thanks to notify us about it after almost 3 years.

 

But why did you revert it a few months later?

Edited by littlesat, 21 August 2023 - 11:23.

[piplan1966]

Hi
It is marked first, then the name and time. How is it written?

[piplan1966]

Edited by littlesat, Today, 15:23.

[Dimitrij]

 

If so then many thanks to notify us about it after almost 3 years.

Read the entire thread carefully and you'll understand

https://forums.openp...es-in-filepush/

[Huevos]

So... But why did you revert it a few months later?

We started with Simon's commit. It worked fine. Later we imported Erik's 17 commits over 2 months. When it was obvious that Erik wasn't making any further changes and it still didn't work we went back to Simon's working version. After all what is the point having code that causes the box to lock up after recording like currently happens on PLi?

[littlesat]

When it was obvious that Erik wasn't making any further changes and it still didn't work we went back to Simon's working version. 

 

Maybe you missed that Erik health was not optimal... for me for example the same... So it is not nice to communicate it here like you're doing now. This was back in the beginning of 2021. So please respect!

Please understand your respond feels very brute! Maybe that clarifies my responds. I was hospitalized at around that time. So this makes this stuff a kind of emotional.

 

e.g. When reverting back does transcode work on VU+ without Stream-relay?

 

When we can resolve it without reverting (we have already some improvements) this is still preferable!!!

 

@Dima,

Does this https://github.com/O...bec0de5d116ed53 revert the stuff required?

Edited by littlesat, 21 August 2023 - 13:09.

[Huevos]

 

When it was obvious that Erik wasn't making any further changes and it still didn't work we went back to Simon's working version. 

 

Maybe you missed that Erik health was not optimal... for me for example the same... So it is not nice to communicate it here like you're doing now. This was back in the beginning of 2021. So please respect!

Please understand your respond feels very brute! Maybe that clarifies my responds. I was hospitalized at around that time. So this makes this stuff a kind of emotional.

I'm not sure what you mean. I am just telling you what happened. I'm not blaming Erik. I reverted because users want their boxes to work properly.

Edited by Huevos, 21 August 2023 - 15:23.

[WanWizard]

Simon confirmed that the recording (and playback) signalling setup didn't work, which leaves the recording thread vulnerable to deadlocks should it ever empty the read buffer. Maybe it worked 5 years ago, but running some tests where he disabled the DMX input into the file used as the source, he observed ::read in eFilePushThreadRecorder::thread() blocking pending some data to read and not responding to SIGUSR1 raised by the main thread.
PR to fix this here: https://github.com/O...nigma2/pull/593

 

That is the same conclusion Erik and I reached. But since signalling is glibc / POSIX, why does it only impact a few people, and not everyone (in 3 years we have exactly two people complaining). And what is blocking the signals?

 

Too bad Simon isn't around to explain why in certain cases the signalling seems to be blocked.

[Dimitrij]

 

Does this https://github.com/O...bec0de5d116ed53 revert the stuff required?

Now I'm not sure of anything.

[Erik Slagter]

I had a look at the relevant patch by Simon. It's interesting, because at first sight indeed it seems to fix a missing an unblocking action and for the code where it's applied, this code indeed is required. But in PLi's version of Enigma, this extra code would not fix anything, because the signal (USR1) is already unblocked in another way. During my own debug sessions I saw the signals being sent, received and acted upon, so I don't think there is an issue there.

 

While Simon takes the approach to selectively add and remove signals to the set of blocked signals, I bluntly set the whole mask to "on" (all signals) and remove USR1 from it, or I do the other way around where required. I do not think a file pusher thread should be bothered by any other signal than this, if anyone disagrees, please speak up and explain.

 

So sadly, this isn't going to solve the problem.

 

Then there is another source of confusion. The code has some references to (eThread::)kill(). It sounds interesting to use this a handbrake to stop the thread if it doesn't stop by itself. It won't work though. This method has a call to pthread_join(), which does nothing other than wait for the thread to finish. If the thread is not planning to finish, you'll have hanging enigma.

 

There is a way to actually stop a thread and that's called "cancelling" it (pthread_cancel). This is not used anywhere in enigma, so if we're going to start using it, we'll have to add it. The process isn't as interesting as it looks though, as by default threads are started "cancel deferred". This means a thread can only be stopped at "cancellation points", these are mostly rescheduling system calls. So if a thread goes completely rogue, you won't be able to stop it that way. It's possible to set the thread to "cancel asynchronous" though. For using it as a last resort that may be interesting. Would need thorough testing though. It's not possible to use pthread_cancel() to stop the thread in normal circumstances (i.e. at the end of the recording session) though, so we'd keep needing the construction with the signals anyway.

 

But adding extra calls to (eThread::)kill() is not a solution, that's should be clear.

Edited by Erik Slagter, 21 August 2023 - 18:08.

[Huevos]

After 4 months of trying to help PLi and coming up against the normal immovable object he eventually gave up and ask me to revert OpenViX to his original working commits. Extremely frustrating. The following were a couple of comments from Simon at the time:

• Erik's going round and round, trying so hard to fix the problem without basing it on my two simple PRs. He's removed the thread signal unblock now, which is wrong, so we've got half a fix, but with more code.
• My approach is that the existing code has been largely unaltered for 7 or 8 years, which is 7 or 8 years' worth of testing, so let's not change it more than we have to.
• Erik has ignored my advice to unblock SIGUSR1 explicitly on the thread as per my initial recommendation in November. I think I'll wait until the user with the XTrend box submits a log with 100 thread stop retries and Erik is really baffled before suggesting it again.
• I'm afraid I got a bit fed up with being a guinea pig for this. PLi weren't interested in taking the actual fix from the guy that had a debugger on a box that was affected and seem to be more interested in fixing bugs that don't actually manifest and introducing more that didn't exist before. It's just not acceptable for the primary function of a PVR to be broken for 4 months.

Edited by Huevos, 21 August 2023 - 18:44.

[WanWizard]

Here's an enigma with Simon's method of signal handing:

 

 enigma2_3.9+git20839+854d615-r0.0_vuduo4kse.ipk   5.57MB   2 downloads

 

I see another difference, which is that in VIX kill() is always and directly executed on stop(), while we have a timeout look which sends the signal up to 100 times, and doesn't kill if the loop ends before the signal is acted upon.

[Stan]

 

[...] I'll wait until the user with the XTrend box submits a log with 100 thread stop retries [...]

 

On my Xtrend I did have hanging Enigma when these changes were made back in January 2021. I have then uninstalled all plugins that can slow down the box (like LCD4Linux).

Now, a typical log, when recording stops, looks like this:

[TIMER] stop recording on tuner: A
[eDVBServiceRecord] stop recording!
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] read got interrupted by signal, stop: 0
[eFilePushThreadRecorder] stopping thread: 100
[eFilePush] SIGUSR1 received
[eFilePushThreadRecorder] read got interrupted by signal, stop: 1
[eDVBRecordFileThread] waiting for aio to complete
[eDVBRecordFileThread] Waiting for I/O to complete
[eFilePushThreadRecorder] stopping thread: 99
[eFilePush] SIGUSR1 received
[eDVBRecordFileThread] aio_suspend failed: Interrupted system call
[eDVBRecordFileThread] Waiting for I/O to complete
[eFilePushThreadRecorder] stopping thread: 98
[eFilePush] SIGUSR1 received
[eDVBRecordFileThread] aio_suspend failed: Interrupted system call
[eDVBRecordFileThread] Waiting for I/O to complete
[eFilePushThreadRecorder] stopping thread: 97
[eFilePush] SIGUSR1 received
[eDVBRecordFileThread] aio_suspend failed: Interrupted system call
[eDVBRecordFileThread] Waiting for I/O to complete
[eFilePushThreadRecorder] stopping thread: 96
[eFilePush] SIGUSR1 received
[eDVBRecordFileThread] aio_suspend failed: Interrupted system call
[eDVBRecordFileThread] buffer usage histogram (20 buffers of 188 kB)
[eDVBRecordFileThread]   0:      5
[eDVBRecordFileThread]   1:    510
[eDVBRecordFileThread]   2:     45
[eDVBRecordFileThread]   3:     17
[eDVBRecordFileThread]   4:      8
[eDVBRecordFileThread]   5:      4
[eDVBRecordFileThread]   6:      3
[eDVBRecordFileThread]   7:      1
[eFilePushThreadRecorder] stopping thread: 95
[eFilePush] SIGUSR1 received
[eFilePushThreadRecorder] THREAD STOP

Edited by Stan, 21 August 2023 - 20:11.

[Huevos]

Here's an enigma with Simon's method of signal handing:

 

enigma2_3.9+git20839+854d615-r0.0_vuduo4kse.ipk

 

I see another difference, which is that in VIX kill() is always and directly executed on stop(), while we have a timeout look which sends the signal up to 100 times, and doesn't kill if the loop ends before the signal is acted upon.

The 100 tries was added to try to simulate what OpenViX does immediately, but anyway it doesn't work and the box freezes. In Simon's case it was the ET8500.

[Stan]

Why has this code, that goes very deep in the recording process, not been reserved only for the boxes it was ment for? (VU+?)

Why did it have to be forcefully applied also to boxes like the Xtrends?

Edited by Stan, 21 August 2023 - 21:18.

[WanWizard]

Try maintaining (developing, testing) code that is littered with IF BOX THEN blocks for every box that is supported...