99674 2012-10-17 19:59:25 -0700 Web Inspector: REGRESSION: [JSC] SourceProvider reuses IDs 2013-02-26 16:45:44 -0800 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 timothy oliver apavlov fpizlo ggaren keishi loislo oliver pfeldman pmuellr roger_fong vsevik web-inspector-bugs webkit-bug-importer yurys oldest_to_newest 744917 0 timothy 2012-10-17 19:59:25 -0700 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> 744919 1 timothy 2012-10-17 20:14:31 -0700 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. 744956 2 ggaren 2012-10-17 21:54:38 -0700 We could also have JSC provide a UUID instead of a pointer. 744970 3 timothy 2012-10-17 22:05:15 -0700 Yeah, that was option two I mentioned in the description: "or SourceProvider needs a new thread safe unique ID assigned when SourceProvider is created." 749765 4 timothy 2012-10-24 10:11:38 -0700 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. 819098 5 oliver 2013-01-29 10:57:48 -0800 (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? 819106 6 timothy 2013-01-29 11:06:11 -0800 (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. 819109 7 oliver 2013-01-29 11:08:25 -0800 (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. 819133 8 timothy 2013-01-29 11:32:47 -0800 If there isn't an issue with two threads grabbing the same ID, then nothing. 842498 9 190388 oliver 2013-02-26 16:30:31 -0800 Created attachment 190388 Patch 842506 10 oliver 2013-02-26 16:40:00 -0800 Committed r144122: <http://trac.webkit.org/changeset/144122> 842511 11 roger_fong 2013-02-26 16:45:44 -0800 Broke windows build: http://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/44992/steps/compile-webkit/logs/stdio 190388 2013-02-26 16:30:31 -0800 2013-02-26 16:33:36 -0800 Patch bug-99674-20130226162645.patch text/plain 3028 oliver U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ0MTE4CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBm ZDI1ZTg1Mzk3MjA1OTlkYzA3NThlYmZiOWVkYmFmMzg2NTNhYjYxLi43NmE3NmVjOTgwMzRmZDJh NzMyODRlYmJkYzFhZWFlMzcwOGY4MDA1IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMiBAQAorMjAxMy0wMi0yNiAgT2xpdmVyIEh1bnQgIDxvbGl2ZXJAYXBwbGUuY29tPgor CisgICAgICAgIFdlYiBJbnNwZWN0b3I6IFJFR1JFU1NJT046IFtKU0NdIFNvdXJjZVByb3ZpZGVy IHJldXNlcyBJRHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTk5Njc0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgU2ltcGxlIGluY3JlbWVudGluZyBjb3VudGVyIGZvciBTb3VyY2VQcm92aWRlciBJRHMuICBV c2VzIGEKKyAgICAgICAgbG9jayB0byBpbmNyZW1lbnRpbmcgdGhlIGNvdW50ZXIgc28gd2UgZG9u J3QgaW5jcmVtZW50IHJldXNlCisgICAgICAgIGNvdW50ZXIgdmFsdWVzIG9yIHJlYXNzaWduIHRo ZSBJRCBmb3IgYSBnaXZlbiBTb3VyY2VQcm92aWRlci4KKworICAgICAgICAqIHBhcnNlci9Tb3Vy Y2VQcm92aWRlci5jcHA6CisgICAgICAgIChKU0M6OlNvdXJjZVByb3ZpZGVyOjpTb3VyY2VQcm92 aWRlcik6CisgICAgICAgIChKU0MpOgorICAgICAgICAoSlNDOjpTb3VyY2VQcm92aWRlcjo6Z2V0 SUQpOgorICAgICAgICAqIHBhcnNlci9Tb3VyY2VQcm92aWRlci5oOgorICAgICAgICAoSlNDOjpT b3VyY2VQcm92aWRlcjo6YXNJRCk6CisgICAgICAgIChTb3VyY2VQcm92aWRlcik6CisKIDIwMTMt MDItMjYgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgogCiAgICAg ICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHIxNDQwNzQuCmRpZmYgLS1naXQgYS9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvcGFyc2VyL1NvdXJjZVByb3ZpZGVyLmNwcCBiL1NvdXJjZS9KYXZhU2Ny aXB0Q29yZS9wYXJzZXIvU291cmNlUHJvdmlkZXIuY3BwCmluZGV4IGM5OThkMTI0ZGY3ZjM2MDY4 Zjk1Mzg2YWYxMDdlY2EzZmM0NTA4ZTAuLjFlYzBlZTRlZmYwZTdmOGYxYzIxNTRkYTM2OGJlMDEx YjBjODU1OTMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIvU291cmNl UHJvdmlkZXIuY3BwCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIvU291cmNlUHJv dmlkZXIuY3BwCkBAIC0yNSw2ICsyNSw3IEBACiAKICNpbmNsdWRlICJjb25maWcuaCIKICNpbmNs dWRlICJTb3VyY2VQcm92aWRlci5oIgorI2luY2x1ZGUgPHd0Zi9UQ1NwaW5Mb2NrLmg+CiAKIG5h bWVzcGFjZSBKU0MgewogCkBAIC0zMiw2ICszMyw3IEBAIFNvdXJjZVByb3ZpZGVyOjpTb3VyY2VQ cm92aWRlcihjb25zdCBTdHJpbmcmIHVybCwgY29uc3QgVGV4dFBvc2l0aW9uJiBzdGFydFBvc2l0 CiAgICAgOiBtX3VybCh1cmwpCiAgICAgLCBtX3N0YXJ0UG9zaXRpb24oc3RhcnRQb3NpdGlvbikK ICAgICAsIG1fdmFsaWRhdGVkKGZhbHNlKQorICAgICwgbV9pZCgwKQogewogfQogCkBAIC0zOSw1 ICs0MSwxNiBAQCBTb3VyY2VQcm92aWRlcjo6flNvdXJjZVByb3ZpZGVyKCkKIHsKIH0KIAorc3Rh dGljIFRDTWFsbG9jX1NwaW5Mb2NrIHByb3ZpZGVySWRMb2NrID0gU1BJTkxPQ0tfSU5JVElBTEla RVI7CisKK3ZvaWQgU291cmNlUHJvdmlkZXI6OmdldElEKCkKK3sKKyAgICBTcGluTG9ja0hvbGRl ciBsb2NrKCZwcm92aWRlcklkTG9jayk7CisgICAgaWYgKCFtX2lkKSB7CisgICAgICAgIHN0YXRp YyBpbnRwdHJfdCBuZXh0UHJvdmlkZXJJRCA9IDA7CisgICAgICAgIG1faWQgPSArK25leHRQcm92 aWRlcklEOworICAgIH0KK30KKwogfSAvLyBuYW1lc3BhY2UgSlNDCiAKZGlmZiAtLWdpdCBhL1Nv dXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIvU291cmNlUHJvdmlkZXIuaCBiL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9wYXJzZXIvU291cmNlUHJvdmlkZXIuaAppbmRleCBkZWMzODVhNjRhZGQ2NzNk ZDM3YWFiNmQzYTViOTQ5NzczMDI2N2NkLi45MjlkYzQ2ZjQwMjA4NWY4NmY5OTJlYmFkODMyMDc4 ODk2Y2JkNTYxIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcGFyc2VyL1NvdXJj ZVByb3ZpZGVyLmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3BhcnNlci9Tb3VyY2VQcm92 aWRlci5oCkBAIC01OCw3ICs1OCw5IEBAIG5hbWVzcGFjZSBKU0MgewogICAgICAgICAgICAgQVNT RVJUKHRoaXMpOwogICAgICAgICAgICAgaWYgKCF0aGlzKSAvLyBCZSBkZWZlbnNpdmUgaW4gcmVs ZWFzZSBtb2RlLgogICAgICAgICAgICAgICAgIHJldHVybiBudWxsSUQ7Ci0gICAgICAgICAgICBy ZXR1cm4gcmVpbnRlcnByZXRfY2FzdDxpbnRwdHJfdD4odGhpcyk7CisgICAgICAgICAgICBpZiAo IW1faWQpCisgICAgICAgICAgICAgICAgZ2V0SUQoKTsKKyAgICAgICAgICAgIHJldHVybiBtX2lk OwogICAgICAgICB9CiAKICAgICAgICAgYm9vbCBpc1ZhbGlkKCkgY29uc3QgeyByZXR1cm4gbV92 YWxpZGF0ZWQ7IH0KQEAgLTY2LDkgKzY4LDEyIEBAIG5hbWVzcGFjZSBKU0MgewogCiAgICAgcHJp dmF0ZToKIAorICAgICAgICBKU19FWFBPUlRfUFJJVkFURSB2b2lkIGdldElEKCk7CisKICAgICAg ICAgU3RyaW5nIG1fdXJsOwogICAgICAgICBUZXh0UG9zaXRpb24gbV9zdGFydFBvc2l0aW9uOwot ICAgICAgICBib29sIG1fdmFsaWRhdGVkOworICAgICAgICBib29sIG1fdmFsaWRhdGVkIDogMTsK KyAgICAgICAgdWludHB0cl90IG1faWQgOiBzaXplb2YodWludHB0cl90KSAqIDggLSAxOwogICAg IH07CiAKICAgICBjbGFzcyBTdHJpbmdTb3VyY2VQcm92aWRlciA6IHB1YmxpYyBTb3VyY2VQcm92 aWRlciB7Cg==