Click here to go to the forum index Click here for the home page
 
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 Reply with quote
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
View user's profile Send private message Visit poster's website
bellissimo
Posted: Wed Apr 23, 2008 9:04 am Reply with quote
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
View user's profile Send private message Send e-mail Visit poster's website
R2-D2
Posted: Wed Apr 23, 2008 9:22 am Reply with quote
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
View user's profile Send private message Visit poster's website
bellissimo
Posted: Wed Apr 23, 2008 9:26 am Reply with quote
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

_________________
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
View user's profile Send private message Send e-mail Visit poster's website
R2-D2
Posted: Wed Apr 23, 2008 9:45 am Reply with quote
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
View user's profile Send private message Visit poster's website
BobD
Posted: Wed Apr 23, 2008 12:48 pm Reply with quote
MyStuff Team Joined: 03 Aug 2005 Posts: 4187 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!), and recommended support TAPS
MyStuff skins, manual and latest version: http://www.BobDsMyStuff.co.uk
Known bugs & forthcoming fixes: http://www.BobDsMyStuff.co.uk/Bugs.shtml
Changes coming in the next version: http://www.BobDsMyStuff.co.uk/NextVersion.shtml
View user's profile Send private message Visit poster's website
R2-D2
Posted: Wed Apr 23, 2008 12:55 pm Reply with quote
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
View user's profile Send private message Visit poster's website
R2-D2
Posted: Wed Apr 23, 2008 1:06 pm Reply with quote
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
View user's profile Send private message Visit poster's website
simonc
Posted: Wed Apr 23, 2008 1:57 pm Reply with quote
Frequent contributor Joined: 12 Apr 2005 Posts: 5617 Location: Cheltenham
I don't recall anyone complaining about the playback stopped message until quite recently, so I think you're safe.
View user's profile Send private message Visit poster's website
adi
Posted: Sat May 03, 2008 10:58 am Reply with quote
Frequent contributor Joined: 24 May 2006 Posts: 118 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.5; EIT Sub v0.6; EPG2MEI v0.96; Font Manager 1.0; MyInfo B5.6; SecCache (UK) v0.4; TAP Commander 1.34; TSSaver v0.5; UK Subtitle 1.9; TF5000 Display v1.53; (FastScanGUI v0.6d);
Sig generated by MyInfo on 2/7/13
View user's profile Send private message
R2-D2
Posted: Wed Jan 28, 2009 2:05 pm Reply with quote
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
View user's profile Send private message Visit poster's website
R2-D2
Posted: Wed Jan 28, 2009 2:47 pm Reply with quote
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
View user's profile Send private message Visit poster's website
R2-D2
Posted: Wed Jan 28, 2009 5:09 pm Reply with quote
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
View user's profile Send private message Visit poster's website
simonc
Posted: Wed Jan 28, 2009 5:42 pm Reply with quote
Frequent contributor Joined: 12 Apr 2005 Posts: 5617 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?
View user's profile Send private message Visit poster's website
R2-D2
Posted: Wed Jan 28, 2009 6:25 pm Reply with quote
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
View user's profile Send private message Visit poster's website

Display posts from previous:  

All times are GMT
Page 1 of 2
Goto page 1, 2  Next

Jump to:  

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