IRC Log Viewer » #firebreath » 2010-09-09

IRC Nick Time (GMT-7) Message
jack_thompson 08:09 hmm... i have a plugin that compiles fine in linux and windows... but about:plugins of frefox/windows doesn't show it...
i renamed it to npudps32.dll
ad put it in the right place.. c:\program files\mozilla ...\plugins\
jack_thompson 08:09 works now
taxilian 09:09 jack_thompson: you still around?
amackera 09:09 Evidently not :P
taxilian 09:09 guess not =]
he'll come back eventually
so amackera, when are you going to get that merge done for me? =]
amackera 09:09 :P
I'll do it right now, if I get caught up in my stuff I'll never get a chance
taxilian 09:09 hehe. I appreciate it
amackera 09:09 It's the taxilian-misc clone? or trunk?
taxilian 09:09 I'll then merge it into trunk, and you won't have to do it again
amackera 09:09 k
taxilian 09:09 taxilian-misc has been merged into trunk already
amackera 09:09 k
taxilian 09:09 today if I can I will pull in the security zones stuff
amackera 09:09 So just to be clear the plan is for me to merge into trunk and commit?
taxilian 09:09 merge from trunk into your branch
I'll pull it into trunk
amackera 09:09 Ok I see
taxilian 09:09 that way a second pair of eyes will go over the merge before it goes into head
I'd love to have that always be the standard
but usually there is nobody available to review my code when I push =]
amackera 09:09 Haha yeah I hear you
I want so badly to have regular code reviews at work, but nobody apart from me has any C/C++ experience
Or time, for that matter, heh
taxilian 09:09 yeah
kalev 09:09 yeah, I would love to have someone go over my code too, but nobody ever does
taxilian 09:09 Georg used to go over anything I committed, and I pretty much review anything that goes into FireBreath, but everyone is so busy, it's hard to get a good code review anymore
it would also be interesting sometime to do some real heavy testing with Valgrind to see if there are any memory leaks in FireBreath
kalev 09:09 oh, that reminded me something
not sure if you know, but I work with Antti Andreimann. He contributed a few fixes to firebreath some time ago.
he hasn't worked on the plugin project for a while, but last time he did he came up with a (in my opinon) incredible hack to work around a bug somewhere in NPAPI retain/release
I don't really know much about NPAPI
we were seeing crashes both in Windows and Linux with plugin unloading
and this patch makes the crashes disappear at least
what's interesting though is that the unloading crashes didn't happen with Firefox's new out of process plugin implementation; only when the plugin was running in browser's process
amackera 09:09 Probably was fixed as a side-effect of hosting the plugin's own process
jack_thompson 09:09 taxillian, back!
amackera 09:09 Seems like it's a bug in RetainObject(), not actually retaining objects :P
jack_thompson 09:09 ah..
kalev 10:09 yeah :P
amackera 10:09 kalev: that's a neat hack/workaround
taxilian: I've merged trunk into my clone
taxilian 10:09 kalev: sorry, I was away for a bit. Let me look over Antti's hack; yeah, I remember him
jack_thompson: welcome back =]
jack_thompson: this plugin that you're having problems with; is it a firebreath plugin or a straight npapi plugin?
kalev: for that doubleretain thing, can you tell me which browsers that has been seen on?
kalev 10:09 taxilian: hold on, I'll call Antti and ask him if he remembers; Firefox was one of them for sure but I think there were more
taxilian 10:09 I know there are a couple of issues on this
but I want to make sure I am clear on where they stand
kalev: If there is any way that antti could pop in for a second, I have a couple of other questions.
for example, I don't understand why he would check for referenceCount < 1; if NPAPI is implemented correctly by the browser, ReleaseObject will automatically free the object if the reference count is < 1; so the second check should be at best redundant, at worst could cause undefined behavior (since the memory it is checking should be freed)
jack_thompson 10:09 taxilian still npapi...
i am fighting my way through the vc++ howto....
taxilian 10:09 jack_thompson: ok; I'll omit the obvious suggestion about "in firebreath this always works", then ;-) what crash are you experiencing?
jack_thompson 10:09 i should have tried that before i "cleaned it up"
taxilian 10:09 what version of visual studio are you using?
jack_thompson 10:09 oh... vc++6.0 xD
it the only vc++ i could get so quick
the plugin is detected and the page loads...
i'll try it with npruntime first...
i mean the howto...
to see if the old vc++ is the problem...
kalev 10:09 taxilian: talked with Antti. He wasn't at computer right now but promised to come to this channel in an hour or maybe two.
taxilian 10:09 jack_thompson: well, that would definitely make it hard to use firebreath, then =] there is no reason you couldn't do npapi with that version
kalev: ok; I will probably be on then. I'm juggling school homework and work right now =]
kalev 10:09 perfect.
taxilian 10:09 jack_thompson: the plugin detected means you have your string resources correct; it crashes when the page loads?
oh, when you try to script it
got it
sorry, didn't see the pm
make sure you're returning the object from NPP_GetValue correctly
it's not going to be a VC++ 6 issue
amackera 10:09 Anybody have any good recommendations for intro to TDD or unit testing
taxilian 10:09 hmm. not here =] you could look over how the ones in FireBreath are written
you could even expand them for practice, if you want!
I'll be back in a bit; gotta move. (currently sitting on a bench in a random hallway)
jack_thompson 10:09 aparrently vc++ 6.0 seems to be the problem... :o
but i dont see myself reinstalling windows on a larger virtualbox image... and then finding visualc net something..
i just ignore windows :D
taxilian 10:09 jack_thompson: why do you say that vc++ 6.0 is the problem?
jack_thompson 10:09 because the npruntime sample doesn't work either...
it doesn't quit firefox.. but it freezes...
i dont know how else to verify i can build plugins in windows
i'll just release the plugin for linux and postpone the windows port infinitely... i will not allow win32 to get into my way of doing this
maybe someone else fixes it when i add the project files
taxilian 11:09 if you use firebreath, I could probably take a look at it when you have the core working =]
(yes, I'm trying to get more open source examples to use firebreath)
the hardest part of getting a new plugin working with FireBreath is getting python working to generate the skeleton project and getting cmake installed
jack_thompson 11:09 do plugins that use firebreath have python as dependency later? or is that just firebreath?
taxilian 11:09 no
that's just used to generate the initial project
you could do it by hand if you want
just faster to use python
(it uses the cheetah template engine to process the example files, generate your guids for activex, set your mimetypes, etc)
jack_thompson 11:09 ah... kay..
taxilian 11:09 yeah, I wouldn't go for a python dependency either =]
here is the template it uses:
jack_thompson 11:09 and i get a shared library... people dont need to install firebreath to use the plugin right?
taxilian 11:09 right
firebreath is a framework
jack_thompson 11:09 k...
i'll give it a try... my head is empty after clicking on so many stupid windows buttons... i'll watch some tv or so...
taxilian 11:09 lol
could be worse
I'm doing binary math on paper
jack_thompson 11:09 :]
i'll bother you again when i run into problems :D
taxilian 11:09 ok
if I'm not here, the other guys can most likely help you as well
jack_thompson 11:09 k :)
taxilian 11:09 oh, and a general FYI; I am always signed on, even if I'm not actually connected; I'll see any messages when I get back. That's why sometimes it takes me so long to answer
amackera 12:09 brb, stupid apple updates require restart, wtf!?
kalev 12:09 taxilian: talked to Antti again on the phone and he sounded like he might not get to computer any time soon
maybe tomorrow then.
taxilian 12:09 kalev: ok; I'd be interested to know more about his findings, but I'll look at it. I might do something similar in FireBreath to protect against that edge case; it seems unlikely that it could cause problems, with the exception of the innermost if statement that seems a little strange
amackera 12:09 Oh knows taxilian, after merging to trunk all my terrible, terrible hacks don't work!
oh noes*
taxilian 13:09 amackera: what terrible hacks don't work?
amackera 13:09 CAOpenGLLayer doesnt work in Safari anymore for some reason
I think i had a hacked that i reverted
aha! but i made a backup beforehand!
kalev 13:09 mercurial makes backups automatically if you revert something
amackera 13:09 Genius
taxilian 13:09 … and you can always go look in the history =]
kalev 13:09 no. in git yes, revert reverts already commited changesets and you can look at the history, but in mercurial revert just undoes changes to the working tree.
mercurial revert <file> == git checkout -- <file>
taxilian 13:09 are you sure? I could swear I've used revert and it just pulled out the old version
ahh, well
kalev 13:09 oh could be, maybe it does two different things
taxilian 13:09 sounds like he's got it covered
kalev 13:09 I'm no mercurial expert :)
taxilian 13:09 yeah, me neither
still debating if it's worth the trouble of moving firebreath to github
kalev 13:09 your call. I personally prefer git over mercurial, but it's also nice to keep both version control and bug tracking / wiki at one place. Furthermore, it's probably extra work for people who have checkout out firebreath mercurial repo. Not sure how many there are, perhaps most people use either nightly snapshots or the released tarballs.
taxilian 13:09 right
that's the question
in a lot of ways, git would be much easier
and I could keep the wiki and the files where they are
amackera 13:09 Turns out the breakage to my awful no-good hacks was entirely my fault (honestly was there any doubt?)
taxilian 13:09 hehe
well, as long as you can fix them
anttix 14:09 Hi!
I was notified by Kalev that there is an interest in my "double - trouble - retain - or - die" hack ;)
I am not sure the hack is a good thing, othervise I would of pushed it to the source tree.
However applying it does reveal a number of deficiencies in Firefoxs' NPAPI.
The problems are only visible when event handlers are used.
When destroying the list that holds the event handler JSObjects, some objects show invalid reference counts, and the browser sometimes crashes.
BTW there is an if that checks if reference count is zero in Firefox code and refuses to free if so ;) It's an ugly hack that valgrind grunts about and it seems to me that reference counting is not very stable in FF.
We have seen crashes when unloading the plugin object, even with doubleretain patch applied.
I personally have witnessed an occasional crash in FF, our Mac testers told me that Safari crashes even more often.
No crashes with new FF out-of-process code.
I have some working theories about the crashes, but I haven't had any time to verify them :(
The plugin also crashes on unload if there is some other JSO retained in the JSAPI code. Eg. a plugin allows one to set a callback function.
We used to do that in our plugin.
I have a feeling that Firefox and Safari destroy JS objects in another thread at the same time as the main thread destroys the plugin and thus there is a race condition. The crashes usually show a value correspondig to a signed "-1" as the referenceCount (the variable itself is an unsigned int).
taxilian 14:09 anttix: I am back now =]
reading your messages
anttix 14:09 My doubleretain hack shows no errors with chromium or arora, but it will show that reference counts are off when running in FF.
taxilian 14:09 anttix: were you using multiple threads anywhere?
anttix 14:09 nope.
I have even managed to crash the FBTestPlugin :P
taxilian 14:09 what platform(s)?
anttix 14:09 You need to add event handlers in test HTML for it to happen.
Fedora Linux
taxilian 14:09 only linux?
never on windows or mac?
anttix 14:09 The crashes with our plugin have been visible on windows FF and Mac Safari too.
I did not test FBTestPlugin on windows
taxilian 14:09 hmm. I wonder if we could be doing something wrong there
anttix 14:09 The reproducibility is quite bad. I get one crash in 7-10 loads/unloads.
taxilian 14:09 hmm. too bad I didn't have time to work with you on it when you were playing with it
anttix 14:09 And it usually requires me to restart the browser to try again, eg. it seems to break more easily when it's memory is fresh.
taxilian 14:09 hmm
so I'm looking at
anttix 14:09 It seems to me that reproducibility is a bit better with Safari but I do not have a Mac to play with right now.
taxilian 14:09 on line 28, why do you have that check there?
anttix 14:09 Actually I have never hit that test.
taxilian 14:09 you may have
you might not know
it would crash
anttix 14:09 It was there just in case to detect race conditions, but I haven't catched one
Well debugger would of shown me that then.
taxilian 14:09 when you call ReleaseObject, if it goes to 0, it will delete the object
race conditions *should* never happen, because this *should* never be called on another thread
so I'm glad to hear you haven't hit it =]
anttix 14:09 And reading the same memory afterwards will only utter a warning under valgrind :P
taxilian 14:09 ok
anttix 14:09 FYI the Firefox code is full of checks like that. Valgrind will grunt about reading freed memory all the time :P
taxilian 14:09 yeah, I know
that's the next thing I was thinking it would be interesting to try, if I can reproduce the issue
build firefox from source
and log the retains / frees
anttix 14:09 Well, it might shed some light to the issue.
taxilian 14:09 yeah
this feels like it should be a plugin issue
since you'd think that if other plugins had this issue, it would have come up, y'know?
anttix 14:09 At first I think You should test if You can get the same "buggy browser detected" warnings from my patch as I will get.
Those are 100% reproducible with my setup.
Crashes are not that common. That's why I think there must be a race somewhere.
taxilian 14:09 ok; I'll have to play with it. it's easy to test on mac, not so easy on windows
anttix 14:09 Yeah, just make sure You register some event handlers from the test page.
taxilian 14:09 ok; it might be next week before I get to that
anttix 14:09 It only happens when plugin retains some JS objects.
taxilian 14:09 but I appreciate the info
anttix 14:09 I wish I had something more conclusive :(
taxilian 14:09 yeah. very strangeish
that's okay, it's more than I had
anttix 14:09 Right now all I have is a bunch of hunches and some controversial test results.
taxilian 14:09 I've been unable to do any real work on firebreath for 6 months; it's good to have some help on some of the problems found in that time =]
anttix 14:09 Firefox reference counting will be off when using event handlers, that I can tell for sure.
But it shouldn't cause a crash.
taxilian 14:09 well, that's definitely better than crashing
anttix 14:09 Because there is if(referenceCount > 0) free.... in FF code.
taxilian 14:09 so I'll probably at least put some of this in there
you're kidding
*shakes head*
anttix 14:09 The firefox releaseobject actually does this check.
taxilian 14:09 it's been awhile since I dug through that
anttix 14:09 I looked at the code, I don't remember the file name from the top of my head.
taxilian 14:09 I'll definitely need to look at that; if we find that this is the only way to make it reliable, we'll put it into firebreath
reliability is the key thing
more important than smallish leaks
anttix 14:09 yeah, but ....
taxilian 14:09 right
anttix 14:09 My patch didn't make the crashes disappear :(
because sometimes referenceCount will hit -1 :(
taxilian 14:09 so you ran firebreath through valgrind a fair bit, huh? does it seem to be pretty good leak-wise?
anttix 14:09 But since it's unsigned integer, it will overflow.
Pretty good, yeah ;)
taxilian 14:09 well, I'm tempted to just call retain 10 times to be safe :-/
anttix 14:09 I was quite happy. The leaks we detected were mainly from openssl and stuff like that.
taxilian 14:09 considering that this is my 3rd real c++ project that I built from scratch, and the 2nd one had a lot of leaks in it, that's not bad
anttix 14:09 :P
taxilian 14:09 the 2nd one was my first complete plugin framework
firebreath is better
anttix 14:09 FB is cool. I like it :) Othervise we would of invented our own cross-browser IDL compiler :P
taxilian 14:09 I'm glad you like it =] it will be getting an infusion of new features in the next bit as well
because Facebook has hired me to build them a plugin =]
anttix 14:09 cool.
taxilian 14:09 that's where the UTF8 conversion stuff I just put in came from, actually
anttix 14:09 As soon as You don't provide any event handlers it will be rock-stable ;)
taxilian 14:09 lol
anttix 14:09 soon = long :P
taxilian 14:09 yeah, I got that =]
anttix 14:09 anyways, I wish You good luck in tracing that madness.
taxilian 14:09 thanks. I appreciate your input
anttix 14:09 I might lend You a hand if I get some time, but right now I'm quite busy.
We worked around the issues in FF by running our plugin in this new out-of-process container.
References are still off a bit, but no crashes so far.
My running theory is that firefox starts to destroy JS functions in one thread and plugins in the main thread.
taxilian 14:09 shouldn't be
firefox has checks in debug mode everywhere for thread id
throws a warning if anything calls retain or release from a different thread
the thing that makes me doubt this whole thing, though
anttix 14:09 Well, for some ugly reason the functions passed as event handlers to FB code are the main sources of this problem.
taxilian 14:09 is that if the reference count is that far off
why doesn't it crash when we're not involved?
anttix 14:09 I don't know :(
taxilian 14:09 I'm thinking there must be something else going on :-/
anttix 14:09 Anyway, if You have trouble reproducing the problems with reference counts, let me know, I'll do some more tests with my own setup.
Yeah, I was thinking about writing a small plain NPAPI plugin to test the issue, but didn't get too far with the idea.
taxilian 14:09 I have heard that there are lots of ways to mess up an AutoPtr class; I wonder if there are problems with FireBreath's
maybe it's time to look at changing to the boost reference counting ptrs
to eliminate the possibilites
anttix 14:09 That isn't such a bad idea after all ;)
But why are reference counts OK with chromium?
And arora
taxilian 14:09 this is why people hate browser plugins =]
anttix 14:09 Yep. And this is why a framework that will deal with all the braindead NPAPI implementations out there is a darn good idea in the first place ;)
ok, I gotta go now. It's 11:53PM around here :P
taxilian 14:09 hehe
thanks for popping in
if you find time, you might find JUCE interesting to look at
it's the closest thing to a competitor for FireBreath that I've seen
anttix 14:09 I'll check it out. Thanx.
taxilian 14:09 might have some things we can learn from them
anttix 14:09 maybe ;)
taxilian 14:09 I thought you left?
anttix 14:09 Yeah I did ;) My pidgin just reopened the window. Ok, bye for the second time ;)
amackera 15:09 I tried using pidgin for a bit for IRC, pretty much fails as an IRC client
taxilian 15:09 I actually haven't had any problems with it
I'm using adium now
of course, I don't use many fancy irc features =]
hmm. so if I were to change to use the boost shared_ptr, I would not need an embedded reference count in the JSAPI class anymore
could this ever become a problem?
amackera 15:09 Why would it, unless there's bugs in boost
are you hesitant to include boost as a dependency?
kalev 15:09 shared_ptr is going to be included in the next C++ standard by the way
amackera 15:09 whenever that rolls out
kalev 15:09 it's already in tr1:: namespace in gcc
yeah, and it's going to take some more time before compilers start impementing all C++0x functionality
taxilian 15:09 boost is already included
including shared_ptr.hpp, I believe
yeah, it's there
kalev 15:09 it is, I've used it in our plgin
taxilian 15:09 I just didn't know how to use it at the time
that's a pretty far-reaching change, but I think it would help to make sure there are no problems with the reference counting
amackera 15:09 Seems like a better solution than ref counting
taxilian 15:09 it uses atomic refcounts, which could be a good thing
amackera 15:09 Bonus
kalev 15:09 allright, I'm off to bed too now, night!
amackera 15:09 night!
taxilian 15:09 night
taxilian 15:09 hmm
this shared_ptr change will not be backwards compatible for other plugins
I think it needs to be done
but it won't be backwards compatible
amackera 15:09 For breaking changes my opinion generally is: do it, get it out of the way before more people start using the broken old way
if the change makes sense, which in this case it does
taxilian 15:09 yeah, that's what I think too
actually, as I go through here, I can see a lot of possible places where the use of AutoPtr could be causing some problems
this definitely needs to be done
it will take me some time, though
maybe I'll put that in 1.2 as well =]
headed home; be back later
amackera 16:09 k
amackera 18:09 taxilian: I'm getting what looks like a hang on Safari trying to GetArrayValues
taxilian 18:09 with the test plugin?
amackera 18:09 no on my plugin
i'll try with the tester
taxilian 18:09 can you see if it is reproducable on the test plugin?
amackera 18:09 sure
FBTestPlugin in trunk?
taxilian 18:09 yes
boy let me tell you… changing to shared_ptr is requiring a *lot* of small changes
but it's definitely what needs to happen
gotta run; be back in an hour or two
amackera 19:09 ok sounds good
you're not going to like this taxilian... FBTestPlugin crashes on a clean build at FB::wstring_to_utf8()
Firefox 3.6.8, Mac OS X 10.6
Same crash in Safari for Mac OS X 10.6 (same machine)
taxilian 20:09 hmm. ok
I'll look into it
do you know where in the function?
and/or what the value is that was being converted?
amackera 20:09 not sure, i didn't look too far into it
but it happens on page load
taxilian 20:09 ok :-/ I'll look at it
just trying to finish this autoptr conversion in one hit =]
and I'm always much stronger at development on windows
because I have more experience there
amackera 20:09 makes sense
taxilian 20:09 Hmm. I wonder if I should change the FB::PluginEvent objects to use shared_ptr as well
would help prevent memory leaks, I'm thinking
but also makes things a little more complicated
and breaks backwards compatibility further
amackera 20:09 haha, hmm
they are currently ref counted?
taxilian 20:09 Actually; I don't think I'm normally actually allocating those on the heap
I think they're on the stack
so they should be good
amackera 20:09 you're fine then
well i'm headed home for the night
ttyl :)