NEW 54146
User generated events may not fire properly due to garbage collection 
https://bugs.webkit.org/show_bug.cgi?id=54146
Summary User generated events may not fire properly due to garbage collection
Jeremy Orlow
Reported 2011-02-09 14:33:16 PST
In https://bugs.webkit.org/show_bug.cgi?id=54144 I fixed an issue where events weren't firing because the JS wrapper was getting garbage collected before we fired event listeners. The only way to fix this is to find some point where you know no more events will be fired and only let the wrapper get collected after that. This is what many of the ActiveDOMObjects do. The problem is that, as far as I understand it, a user can fire an event on any event target that they wish at any time. It seems as though it'd be very easy for someone to register an event, drop the handle, have a gc happen, and then get another handle (or use some stashed handle of a parent object and use an event that captures/bubbles) and fire an event. I believe that in such a case, the user will not see an event handle as expected. Thoughts?
Attachments
Add attachment proposed patch, testcase, etc.
Andrew Wilson
Comment 1 2011-02-09 14:42:34 PST
I think this is only an issue for cases where it's possible for the developer to obtain a new reference to a pre-existing resource. In the case of IndexedDB, it probably means that you can't ever GC an instance of the DB that has event handlers, because the developer can resurrect a reference at any time (if I understand the situation correctly) and would expect previously-added handlers to get fired. I don't think there are many other cases like this, though. There's the canonical case of some item in the DOM somewhere that needs to live on after you've dropped your explicit reference, but yeah, you can't GC items in the DOM, and once they are out of the DOM you can't resurrect a reference to them.
Jeremy Orlow
Comment 2 2011-02-09 17:17:01 PST
I added a FIXME to https://bugs.webkit.org/show_bug.cgi?id=54144 to note what should be happening in order to handle this case. It'll be hard to know exactly when an IDB object can or can't be resurrected though.
Alexey Proskuryakov
Comment 3 2011-02-10 08:56:57 PST
> use some stashed handle of a parent object and use an event that captures/bubbles I believe that you are talking about nodes in a document tree here. These don't have their wrappers collected as long as they have custom properties or event handles (see isObservableThroughDOM() in JSDOMBinding.cpp). So I don't think that there is a problem here. We certainly have known and unknown cases that aren't handled properly - see e.g. bug 34231. Another example is nodes in undo/redo stack. These are edge cases, but it would be nice to fix them. Unless you are looking for a general solution (and currently, this code isn't quite centralized), separate bugs with specific examples are preferable. It's also likely that JSC and v8 bindings have different sets of issues.
Jeremy Orlow
Comment 4 2011-02-10 11:24:43 PST
(In reply to comment #3) > > use some stashed handle of a parent object and use an event that captures/bubbles > > I believe that you are talking about nodes in a document tree here. These don't have their wrappers collected as long as they have custom properties or event handles (see isObservableThroughDOM() in JSDOMBinding.cpp). So I don't think that there is a problem here. I'm talking about objects like IDBRequest or FileWriter, not nodes or anything in a document tree. > We certainly have known and unknown cases that aren't handled properly - see e.g. bug 34231. Another example is nodes in undo/redo stack. These are edge cases, but it would be nice to fix them. Unless you are looking for a general solution (and currently, this code isn't quite centralized), separate bugs with specific examples are preferable. I cited a specific bug that I'm fixing. It seems like this is a more general issue though. I've been working on APIs like this within WebKit for 2 years and I had no clue these pitfalls existed. My guess is that most other contributors don't as well and others will have the same issues.
Alexey Proskuryakov
Comment 5 2011-02-10 11:29:32 PST
> I'm talking about objects like IDBRequest or FileWriter, not nodes or anything in a document tree. Sorry, I thought that you talked about nodes because of capturing/bubbling behavior that you've mentioned.
Jeremy Orlow
Comment 6 2011-02-10 11:37:15 PST
(In reply to comment #5) > > I'm talking about objects like IDBRequest or FileWriter, not nodes or anything in a document tree. > > Sorry, I thought that you talked about nodes because of capturing/bubbling behavior that you've mentioned. IndexedDB events now capture/bubble. I forgot this was a pretty uncommon thing outside of nodes :-)
Note You need to log in before you can comment on or make changes to this bug.