Toppy.org.uk ~ View topic - TAP_Hdd_StopTs bug -- fixed! (and then more bugs found...)

Author

Message

< TAP and patch development ~ TAP_Hdd_StopTs bug -- fixed! (and then more bugs found...)

Page 1 of 2

Goto page 1, 2 Next

R2-D2

Posted: Wed Apr 23, 2008 8:58 am

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

TAP_Hdd_StopTs is basically a wrapper on the main firmware function, but it does some quick checks first. In some (rare) cases, however, this stupid wrapper doesn't restore $gp, resulting in (most likely) a crash when the TAP next tries to access a global or call another TAP function. [St] StopTsFix fixes this.

I'm going to check over some of the other TAP API functions, but are there any obvious ones (suspected of causing crashes) that should be first on the list?

Edit: it gets worse.

Last edited by R2-D2 on Sun Feb 22, 2009 10:49 am; edited 1 time in total
_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

bellissimo

Posted: Wed Apr 23, 2008 9:04 am

Frequent contributor Joined: 26 May 2005 Posts: 1997 Location: Reading

Good work. That one always annoyed me!

Is there any way this could be incorporated into TAP code instead of a patch?

_________________
General: FW 5.13.65 Patched, Pioneer DVR-530H-S, Harmony 885.
TAPs: Font Manager, Media Manager, Super PiP, MyStuff, EPG2MEI, mei2archive, QuickJump

Media Manager, Font Manager, Super PiP link: http://www.toppy.org.uk/~bellissimo

R2-D2

Posted: Wed Apr 23, 2008 9:22 am

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

bellissimo wrote:

Is there any way this could be incorporated into TAP code instead of a patch?

Yes, I'll make a simple TAPAPIFix wrapper for FireBirdLib after searching for any others, but that'll take a while to filter down to TAPs. The patch will hopefully help developers and those reporting weird crashes in the meantime.

The rare cases where the crash will occur is when the current block isn't set (usually very soon after a jump) or when the current block is very near the start (<30 blocks).

_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

bellissimo

Posted: Wed Apr 23, 2008 9:26 am

Frequent contributor Joined: 26 May 2005 Posts: 1997 Location: Reading

Thats great. I will finally be able to take out the horrible workarounds then Smile

R2-D2

Posted: Wed Apr 23, 2008 9:45 am

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

I think the only other one is in TAP_Channel_Start() if you ask to start the sub service as radio (which most TAPs are unlikely to try to do).

_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

BobD

Posted: Wed Apr 23, 2008 12:48 pm

MyStuff Team Joined: 03 Aug 2005 Posts: 4016 Location: Nottingham

Fantastic! That means that if this patch is installed I can definately get rid of my horrible illegal jump workaround, and if the patch is not installed I can run the wrapper function instead? You are (as I have said before) a star!

_________________
FW: ChunkyWizard Recommended
TAPs: MyStuff (always one version ahead of everyone else!), TF5000 Display, epg2mei
MyStuff skins, manual and latest version: http://www.BobDsMyStuff.co.uk
Known bugs & forthcoming fixes: http://www.BobDsMyStuff.co.uk/Bugs.shtml

R2-D2

Posted: Wed Apr 23, 2008 12:55 pm

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

BobD wrote:

if the patch is not installed I can run the wrapper function instead?

It's worth trying out the patch to see if it cures the problem (or get people reporting a crash to try it). I've sent FireBird an addition to the growing list of TAP API fixes, so if he includes it in FireBirdLib at some point then you ought to be able to just use TAP_Hdd_StopTs() in your code.

_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

R2-D2

Posted: Wed Apr 23, 2008 1:06 pm

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

I just suddenly thought that the extra code in the wrapper might have been included to work around the reported problem. Confused

But that code is in 5.12.01 (which is as far back as my list of firmwares goes), so it seems unlikely to me. What's an earlier firmware that was widely used?

And it occurs to me that there could easily be other bugs at a deeper level, but the reports I've seen have linked the crash to the "Playback Stopped" message not being shown, which would fit with this bug since it wouldn't try to do the firmware routine (and hence show the message) if the tests fail (and that would cause the $gp to be wrong, giving a crash a little later in the TAP).

_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

simonc

Posted: Wed Apr 23, 2008 1:57 pm

Frequent contributor Joined: 12 Apr 2005 Posts: 5616 Location: Cheltenham

I don't recall anyone complaining about the playback stopped message until quite recently, so I think you're safe.

adi

Posted: Sat May 03, 2008 10:58 am

Frequent contributor Joined: 24 May 2006 Posts: 117 Location: Heathfield TX

Simon, are you going to add St to your magical patch decoder soon?
Very Happy

Adrian

Edit: OK, you have now, thanks! Wink

_________________
TF5800, TS On, F/W: MS6 Recommended F/W 12/9/2009 -Vr+C0DEvEzFsHsIPePfPsScUUuUxVdZ
TAPs: QuickJump 1.72; MyStuff 6.3; EIT Sub v0.6; EPG2MEI v0.96; Extend v1.7; Font Manager 1.0; MyInfo B5.5; SecCache (UK) v0.4; Standby v1.8; TAP Commander 1.34; TSSaver v0.5; UK Subtitle 1.9; (FastScanGUI v0.6b);
Sig generated by MyInfo on 18/12/10

R2-D2

Posted: Wed Jan 28, 2009 2:05 pm

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

Hmm... there's more to this bug than just the obvious mistakes in the wrapper code...

I was getting a fairly reliable crash with the version of 5.13.65 I was testing that didn't have PBSiS applied. There were no real clues from the crash trace, apart from a clearly corrupt $gp. The fact it seemed to belong to another TAP points the finger squarely at the (crappy) way the firmware copes with the (unnecessary) fiddling of the $gp for TAP API calls. And PBSiS being a complete cure pointed the finger at the code that generates the "Playback Stopped" message. But I was sure I'd looked at that before and not really noticed anything odd.

Then it occurred to me that the stupid thing might be sending IDLE events to TAPs while the message was up... and lo and behold that's exactly what it does. That's pretty much fatal, meaning that the "current TAP index" gets overwritten pretty thoroughly, and so when the TAP_Hdd_StopTs() call finally returns it will use that wrong index to pick up the wrong $gp for the calling TAP (unless it happened to be the last the TAP in the list, possibly). Rolling Eyes

It's a little more complex than that, though, because the TAPAPIFix is robust and will prevent this scenario, reinstating the correct $gp -- and I'm pretty sure BobD is using that in MyStuff (which was doing the stopping of the playback). So it could be unexpected re-entrancy in MyStuff, or even MyStuff calling TAP_Hdd_StopTs() again. Shocked

Any TAP API call that may end up sending IDLE (or any other) events to TAPs is going to break things rather disastrously. TAPs just don't expect to be re-entered unless they explicitly call TAP_SystemProc().

_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

R2-D2

Posted: Wed Jan 28, 2009 2:47 pm

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

Ick! A cursory analysis shows that these all have the potential to do the same sort of thing:

Code:

TAP_Channel_Move
TAP_Channel_Start
TAP_Channel_Stop
TAP_Channel_StopMainSub
TAP_Channel_SwapMainSub
TAP_EnterNormal
TAP_ExitNormal
TAP_Hdd_ChangePlaybackPos
TAP_Hdd_PlayMp3
TAP_Hdd_PlayTs
TAP_Hdd_StartRecord
TAP_Hdd_StopMp3
TAP_Hdd_StopRecord
TAP_Hdd_StopTs
TAP_SetSystemVar

And that's without looking at lots of other ways similar problems may be caused. Interestingly, some of those above will be due to things like "No Signal" banner messages, as well as more obvious things like PIN code prompts.

_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

R2-D2

Posted: Wed Jan 28, 2009 5:09 pm

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

I'm going to make a massive generalisation but... the whole TAP API is completely shagged! I'm now seeing loads of different routes to this re-entrancy and broken $gp. Especially if you change channels or press buttons on the remote. Sad

_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

simonc

Posted: Wed Jan 28, 2009 5:42 pm

Frequent contributor Joined: 12 Apr 2005 Posts: 5616 Location: Cheltenham

Don't press any buttons on the remote then!

How's it managed to sort of work for all this time then? Is it fixable via TAPAPIFix? Should all TAPs have blocks in their event handlers to prevent re-entrancy?

R2-D2

Posted: Wed Jan 28, 2009 6:25 pm

Frequent contributor Joined: 18 Dec 2006 Posts: 12149

simonc wrote:

How's it managed to sort of work for all this time then? Is it fixable via TAPAPIFix?

The conditions appear to be rare enough, I suppose. But I was getting fairly reliable crashes with playback stopping when I was doing some tests under 5.13.65 (MyStuff Recommended version) and so didn't have PBSiS installed. This is partly because MyStuff pre-empts the native (EOF) stopping and invokes TAP_Hdd_StopTs(), as it also does in response to the Stop button. This is not MyStuff's problem per se -- it's just expecting the TAP API to work... Maybe people who don't use MyStuff (or have it loaded as the last TAP) are much less likely to see any issues, but odd corruptions and subtleties now have yet another possible cause.

Quote:

Should all TAPs have blocks in their event handlers to prevent re-entrancy?

That's part of it, yes. Although there's an issue in there about missing potentially important messages if you do that, so ideally you'd have to queue them. Rolling Eyes

Or, alternatively, completely avoid globals (and statics). The second part is the $gp corruption, which really requires all the TAP API functions to have a better wrapper (something like the TAPAPIFix for TAP_Hdd_StopTs()).

At present, I'd like to understand exactly why I was getting the crash traces that show MyStuff running with the wrong TAP's $gp, since I'm fairly sure it's using the TAPAPIFix code.

(And I've just had a crash when a recording was starting, so I'm going right off 5.13.65. Smile

Although I have just installed a different version of MyStuff v5.62c from BobD, so it'll be interesting to see if my current setup exhibits the same sort of problems once I'm back to the familiarity of 5.13.40. Plus I'll stop typing the wrong addresses in the Command Console...)

_________________
Troubleshooting -- User Manual -- Dark Side of the Matrix: Firmwares and Patches

All times are GMT

View next topic View previous topic Page 1 of 2 Goto page 1, 2 Next Toppy.org.uk Forum Index ~ TAP and patch development

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum