Random Crashing on Nightly Git Compile - Please help diagnose
#16
OK that initial run resulted in it compiling GIT:20120604-6b2bb78. Got an immediate crash as soon as I tried to play a movie. Moving on to the next one.
Reply
#17
I was heading to bed but decided I had to eat first, so I had some time while cooking food.

Last good commit:
https://github.com/xbmc/xbmc/commit/a05c...b3ae43fff7
"Revert "Fix AnnouncementManager.cpp modifying song MusicInfoTag data …"

The commit that broke python:
https://github.com/xbmc/xbmc/commit/688b...90f00d83df
"extend xbmcvfs module with, read, write, seek, close, listdir, mkdirs"


May I also say that DAMN that "extend xbmcvfs" is badly coded. Such messy code.

Not only that, but you'll also want to run the following to fix a compiler-breaking typo (suggesting that the author didn't even compile and test his own patch, because it was another person that stepped in and fixed this typo a day later):
perl -pi -e 's/std:Confusedtrlen/strlen/g' xbmc/interfaces/python/xbmcmodule/xbmcvfsmodule.cpp

Is his patch something that was planned? If not, I'd revert the ENTIRE xbmcvfs modification along with the followup patches related to it and not accept it again without lots of code review.

Edit: Just saw your messages Aenima; yes, your process is correct. Although you can roll the entire config/build/install into one line like: make distclean && ./bootstrap && ./configure --your-flags-here && make -j4 && sudo make uninstall && sudo make install. Then just keep building and testing (enter and leave a video library a few times and it'll crash if it's borked), and type "git bisect bad/good" depending on if the revision worked or not. Finally, if you had to edit some file to be able to build a certain revision, you cannot proceed to the next bisection until you first type "git checkout ." to revert to the unedited state. Anyway this is for your future reference, as the borked commit has been found.

Okay food's ready, time to eat and head to bed. Good luck deciding what to do with the broken commit, guys!
Reply
#18
Wow that was fast.....now get some sleep man!
Reply
#19
Possibly fixed in https://github.com/xbmc/xbmc/commit/b6e1...b1c16d2650
Reply
#20
Why is it a messy code? it's done in he same coding style as everything else...

Also, it compiles and runs just fine on osx, no crashes in about a week. PR has been up on github for 2 or so weeks and no one could be bothered to compile on Linux , it got signed off and vdrfan fixed it in an hour as it was not compiling on Linux.

I can't test it on every platform, that's why the PR was posted....

EDIT: Didn't realize it was you, I get it now Smile ....
Reply
#21
(2012-06-06, 20:24)bobo1on1 Wrote: Possibly fixed in https://github.com/xbmc/xbmc/commit/b6e1...b1c16d2650

Nice catch but not fixed.

(2012-06-06, 20:41)amet Wrote: Why is it a messy code? it's done in he same coding style as everything else...

Also, it compiles and runs just fine on osx, no crashes in about a week. PR has been up on github for 2 or so weeks and no one could be bothered to compile on Linux , it got signed off and vdrfan fixed it in an hour as it was not compiling on Linux.

I can't test it on every platform, that's why the PR was posted....

Well here are just a few of the problems:
* std:Confusedtrlen instead of strlen
* mismatch in several prototypes and their actual definitions, such as InitVFSTypes(bool bInitTypes) vs InitVFSTypes(void)
* still crashing even after prototypes are fixed, meaning there are more bugs
Reply
#22
(2012-06-06, 20:58)john.doe Wrote: Well here are just a few of the problems:
* std:Confusedtrlen instead of strlen
* mismatch in several prototypes and their actual definitions, such as InitVFSTypes(bool bInitTypes) vs InitVFSTypes(void)
* still crashing even after prototypes are fixed, meaning there are more bugs

1 is fixed by vdrfan 3-4 days ago
2 by bobo1on1.... several? Is there more that we didn't see?

3 It still doesn't crash on my side, so let us have the crashreport or whatever it is called on the platform you are on
Reply
#23
(2012-06-06, 19:29)john.doe Wrote: May I also say that DAMN that "extend xbmcvfs" is badly coded. Such messy code.

Unless you intend to start submitting patches or contributing to pull/reqs, how about backing off on the ponking please. Nightlies can be volatile, we try our best but sometimes opps will slip in.
Reply
#24
(2012-06-06, 20:58)john.doe Wrote: * still crashing even after prototypes are fixed, meaning there are more bugs

The fact that reverting the commit makes the crashes go away is not proof that the vfs code is responsible for the crashes.
Reply
#25
Might be fixed in https://github.com/xbmc/xbmc/commit/1a2e...e1667d1b0b
Reply
#26
Last fix I promise https://github.com/xbmc/xbmc/commit/5f56...ff73517313
Reply
#27
(2012-06-06, 21:36)davilla Wrote:
(2012-06-06, 19:29)john.doe Wrote: May I also say that DAMN that "extend xbmcvfs" is badly coded. Such messy code.

Unless you intend to start submitting patches or contributing to pull/reqs, how about backing off on the ponking please. Nightlies can be volatile, we try our best but sometimes opps will slip in.

I know, I felt bad when making those comments yesterday, but did it anyway - being tired is not an excuse but it was my reason. I even anticipated this response; heck you were nicer on me than the verbal lashing I expected. I am sorry for how rudely I said it, although not sorry for what I meant - which is that the patch was pretty poorly tested.

(2012-06-06, 22:16)bobo1on1 Wrote:
(2012-06-06, 20:58)john.doe Wrote: * still crashing even after prototypes are fixed, meaning there are more bugs

The fact that reverting the commit makes the crashes go away is not proof that the vfs code is responsible for the crashes.

It was clearly this patch; it was littered with bugs and directly affected the Python interpreter's state. You are an angel for having taken the time to review the code. Here's a recap (for others) of the things you found:

* std:Confusedtrlen instead of strlen (commit 2dd81fe3)
* mismatch in two prototypes and their actual definitions, namely InitVFSTypes(bool bInitTypes) vs InitVFSTypes() and InitXBMCTypes(bool bInitTypes) vs InitXBMCTypes() (commit 343ab3ba and b6e11c39)
* used a bool type to receive a value from a python argument, when it should have been a char (commit 29ecd05d)
* had an uninitialized char pointer set to be an optional value for a python function, meaning that it wasn't guaranteed to be written to, and then just immediately did a string comparison on the uninitialized char pointer (commit 1a2e1bdf)
* didn't increment the reference counter in python before registering new object (commit 5f561f7d)

The last one was most likely the main cause of the crash, followed by the uninitialized char pointer.

Fantastic work finding these bugs, bobo! You deserve a delicious and moist cake! Nod

Edit: Somehow I forgot to slip in the most important bit - the crashes are gone.
Reply
#28
from now I suggest you subscribe to PR requests so that you can do all your bitching in there

well done bobo1on1 on finding all the issues... Big Grin
Reply
#29
@John.doe: Other than the first one (trivial - should have been picked up in the pull request review but wasn't), and perhaps the third one (also should have been picked up in review, but quite a bit less obvious) the errors are completely non-obvious without a very careful reading of the python API, and even then several are non-obvious. Without any crashes (no crashes were observed on os x, and I don't think any popped up on win32 either) or similarly odd behaviour, they're simply not going to be spotted in code review. Only once something goes wrong and you can reproduce it do you start looking at every little thing that could potentially be the cause.

Lastly, I think bobo1on1 would be happy to point out that amet (the guy on the receiving end of your outburst) was the one who found the last one, the real cause of the problem.

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#30
I don't understand how it ran fine on OS X with the reference counter issue present, but okay it ran fine for him so it's more acceptable now knowing that, and I'm also still really sorry about how rude I was yesterday. I've got an injury that kept me up for so long that my eyes were bloodshot, heh.

Edit: Ah Jonathan we posted at the same time. Nice find then, amet! Smile
Reply

Logout Mark Read Team Forum Stats Members Help
Random Crashing on Nightly Git Compile - Please help diagnose0