Bug 99674 - Web Inspector: REGRESSION: [JSC] SourceProvider reuses IDs
Summary: Web Inspector: REGRESSION: [JSC] SourceProvider reuses IDs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-10-17 19:59 PDT by Timothy Hatcher
Modified: 2013-02-26 16:45 PST (History)
13 users (show)

See Also:

Attachments
Patch (2.96 KB, patch)
2013-02-26 16:30 PST, Oliver Hunt 		barraclough: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2012-10-17 19:59:25 PDT 
JSC::SourceProvider provides an ID to identify scripts. This ID is simply this cast to an intptr_t. If SourceProvider is freed another one can be allocated in the same location and cause the same ID to be reported to the Debugger.

I am able to get the same SourceProvider ID on apple.com by reloading a few times. The IDs a reused during the same page load, so it can really confuse the Inspector given that we use these IDs to identify scripts (especially scripts without URLs.)

Either someone needs to retain the SourceProvider (maybe ScriptDebugListener::Script?) so the ID can't be reused or SourceProvider needs a new thread safe unique ID assigned when SourceProvider is created.

<rdar://problem/12517297>
Comment 1 Timothy Hatcher 2012-10-17 20:14:31 PDT 
Retaining SourceProvider will eat more memory since ScriptDebugListener::Script already duplicates the data provided by SourceProvider. Unless we make ScriptDebugListener::Script be a shell for SourceProvider.
Comment 2 Geoffrey Garen 2012-10-17 21:54:38 PDT 
We could also have JSC provide a UUID instead of a pointer.
Comment 3 Timothy Hatcher 2012-10-17 22:05:15 PDT 
Yeah, that was option two I mentioned in the description: "or SourceProvider needs a new thread safe unique ID assigned when SourceProvider is created."
Comment 4 Timothy Hatcher 2012-10-24 10:11:38 PDT 
I am positive this is a regression. I'm seeing the error all the time now when I didn't at all in Safari 6.
Comment 5 Oliver Hunt 2013-01-29 10:57:48 PST 
(In reply to comment #4)
> I am positive this is a regression. I'm seeing the error all the time now when I didn't at all in Safari 6.

What do you mean by thread safe? Same source on different threads needs to provide different ID?
Comment 6 Timothy Hatcher 2013-01-29 11:06:11 PST 
(In reply to comment #5)
> (In reply to comment #4)
> > I am positive this is a regression. I'm seeing the error all the time now when I didn't at all in Safari 6.
> 
> What do you mean by thread safe? Same source on different threads needs to provide different ID?

Same source on different threads should get the same ID. I just meant something more than a global counter.
Comment 7 Oliver Hunt 2013-01-29 11:08:25 PST 
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > I am positive this is a regression. I'm seeing the error all the time now when I didn't at all in Safari 6.
> > 
> > What do you mean by thread safe? Same source on different threads needs to provide different ID?
> 
> Same source on different threads should get the same ID. I just meant something more than a global counter.

Why would a global counter be insufficient?   2^31 (or 2^63) ids should be more than enough.
Comment 8 Timothy Hatcher 2013-01-29 11:32:47 PST 
If there isn't an issue with two threads grabbing the same ID, then nothing.
Comment 9 Oliver Hunt 2013-02-26 16:30:31 PST 
Created attachment 190388 [details]
Patch
Comment 10 Oliver Hunt 2013-02-26 16:40:00 PST 
Committed r144122: <http://trac.webkit.org/changeset/144122>