97539 2012-09-25 01:54:26 -0700 Broken and incorrect code in FastMalloc.cpp 2012-10-11 08:54:02 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Minor --- 1 mark.toller webkit-unassigned ap barraclough benjamin ggaren paroga webkit.review.bot oldest_to_newest 727415 0 mark.toller 2012-09-25 01:54:26 -0700 In the pthread supported #ifdef branch in Source/WTF/wtf/FastMalloc.cpp, the method TCMalloc_PageHeap::signalScavenger() contains the following code: // m_scavengeMutex should be held before accessing m_scavengeThreadActive. ASSERT(pthread_mutex_trylock(m_scavengeMutex)); if (!m_scavengeThreadActive && shouldScavenge()) pthread_cond_signal(&m_scavengeCondition); There are a couple of problems with this: 1) The TCMalloc code can't be compiled in (without changing the build system/editing the file) if debug is enabled, and the ASSERT() macro is compiled out if DEBUG is not enabled. This means that the 'trylock' code is never compiled. 2) The pthread_mutex_trylock(m_scavengeMutex) code won't compile - as it requires a pointer to the mutex (pthread_mutex_trylock(&m_scavengeMutex);) 3) If the (fixed) code was compiled in, it will either: a) cause a deadlock if the mutex is locked successfully, as it is not unlocked afterwards. b) cause the ASSERT to fire if the mutex is not taken (which can happen). The comment indicates that we should take the mutex before checking m_scavengeThreadActive. However, this will cause us to perform a lock/unlock on the mutex for many delete operations. The worst case scenario for *not* locking the mutex is that we either : 1) signal the condvar more often than required when we get several deletes before the m_scavengeThreadActive is set to true or .... 2) We don't signal the condvar as early as possible, and wait till the next delete... 727437 1 165565 mark.toller 2012-09-25 02:41:51 -0700 Created attachment 165565 Patch 727809 2 165565 ap 2012-09-25 11:20:00 -0700 Comment on attachment 165565 Patch I think that this code should be: - ASSERT(pthread_mutex_trylock(m_scavengeMutex)); + ASSERT(!pthread_mutex_trylock(&m_scavengeMutex)); Does such an assert make more sense? 727829 3 benjamin 2012-09-25 11:38:58 -0700 Part of the explanations you put in bugzilla should be in the Changelog. Your ChangeLog is too terse to be useful. 728285 4 mark.toller 2012-09-26 00:45:59 -0700 (In reply to comment #2) > (From update of attachment 165565 [details]) > I think that this code should be: > > - ASSERT(pthread_mutex_trylock(m_scavengeMutex)); > + ASSERT(!pthread_mutex_trylock(&m_scavengeMutex)); > > Does such an assert make more sense? No, because if that code executes it will either deadlock if the lock is acquired, or assert if it fails... And it can (and does) fail if the TCMalloc_PageHeap::scavengerThread() has locked the mutex, and not yet released it in the pthread_cond_wait() call. As Benjamin stated, I should put some of this detail in the Changelog as well. You *could* change it to an : ASSERT(pthread_mutex_lock(&m_scavengeMutex)); if (!m_scavengeThreadActive && shouldScavenge()) pthread_cond_signal(&m_scavengeCondition); ASSERT(pthread_mutex_unlock(&m_scavengeMutex)); However, if the mutex lock/unlock is *needed* then this code shouldn't be wrapped in ASSERT macros, because they are compiled out in release builds... And if they're not required for release builds, and provide no additional benefit in debug, then why have the overhead at all? 728287 5 mark.toller 2012-09-26 00:48:20 -0700 Sorry, that should have been : ASSERT(!pthread_mutex_lock(&m_scavengeMutex)); if (!m_scavengeThreadActive && shouldScavenge()) pthread_cond_signal(&m_scavengeCondition); ASSERT(!pthread_mutex_unlock(&m_scavengeMutex)); as the pthread_mutex_* calls return 0 on success. 728463 6 165777 mark.toller 2012-09-26 06:03:18 -0700 Created attachment 165777 Patch 728613 7 165777 ap 2012-09-26 08:51:28 -0700 Comment on attachment 165777 Patch I think that you still misunderstand the purpose of this assertion. It is there to ensure that caller has taken the lock, so pthread_mutex_lock should fail, and there is no need to unlock. 728618 8 ap 2012-09-26 08:53:47 -0700 > pthread_mutex_lock should fail, and there is no need to unlock. pthread_mutex_trylock* 728665 9 mark.toller 2012-09-26 09:30:00 -0700 (In reply to comment #7) > (From update of attachment 165777 [details]) > I think that you still misunderstand the purpose of this assertion. It is there to ensure that caller has taken the lock, so pthread_mutex_lock should fail, and there is no need to unlock. OK, if that is indeed the case then : 1) The comment "m_scavengeMutex should be held before accessing m_scavengeThreadActive." is misleading, it should be stating "the caller [of signalScavenger()] should ensure that m_scavengeMutex is locked prior to calling this method." 2) The ASSERT *will* currently fail as the only location which locks the mutex (other than this 'trylock') is the scavengerThread itself (TCMalloc_PageHeap::scavengerThread()) - so once the scavengerThread waits on the condvar (and the mutex is then unlocked), the next call to signalScavenger() *will* obtain the lock with the trylock and therefore trigger the ASSERT. I'm not sure if your comment is based on a misunderstanding of the pthread_cond_wait(&m_scavengeCondition, &m_scavengeMutex); code - this atomically waits on the condvar and unlocks the mutex. 728676 10 ap 2012-09-26 09:41:01 -0700 > 1) The comment "m_scavengeMutex should be held before accessing m_scavengeThreadActive." is misleading, it should be stating "the caller [of signalScavenger()] should ensure that m_scavengeMutex is locked prior to calling this method." Yes, I like your variant more. I'd have said "taken" and not "locked", but either works for me. > I'm not sure if your comment is based on a misunderstanding of the pthread_cond_wait(&m_scavengeCondition, &m_scavengeMutex); code - this atomically waits on the condvar and unlocks the mutex. I wrote this code a few years ago, and that the logic was supposed to match what's in the version that is actually used: ALWAYS_INLINE void TCMalloc_PageHeap::signalScavenger() { ASSERT(pageheap_lock.IsHeld()); ... Of course, being an unused branch, it was not tested, and didn't even compile. It's quite possible that a major cleanup of the algorithm is needed there. 729495 11 mark.toller 2012-09-27 07:01:34 -0700 (In reply to comment #10) > > 1) The comment "m_scavengeMutex should be held before accessing m_scavengeThreadActive." is misleading, it should be stating "the caller [of signalScavenger()] should ensure that m_scavengeMutex is locked prior to calling this method." > > Yes, I like your variant more. I'd have said "taken" and not "locked", but either works for me. > > > I'm not sure if your comment is based on a misunderstanding of the pthread_cond_wait(&m_scavengeCondition, &m_scavengeMutex); code - this atomically waits on the condvar and unlocks the mutex. > > I wrote this code a few years ago, and that the logic was supposed to match what's in the version that is actually used: > > ALWAYS_INLINE void TCMalloc_PageHeap::signalScavenger() > { > ASSERT(pageheap_lock.IsHeld()); > ... > > Of course, being an unused branch, it was not tested, and didn't even compile. It's quite possible that a major cleanup of the algorithm is needed there. Ok, I see. Actually, this branch isn't unused as such - it is used by WebkitGtk (GtkLauncher), but as I said earlier, without modifying the build/code, you can't compile using TCMalloc AND debug (so the ASSERT was never compiled in). So, in the GTK/linux variant (no DISPATCH_H and !Windows) a condvar is used to make the scavenger thread sleep. As the condvar requires a mutex, it would seem logical to use the mutex to protect m_scavengeThreadActive - however, the mutex exists only for this convar / thread, and isn't locked (like the pageheap_lock) on entry to 'signalScavenger'. I doubt the overhead of locking/unlocking the mutex on every delete call is desirable? So, how about using the pageheap_lock spinlock to protect m_scavengeThreadActive? The mutex becomes purely a side-effect of using the condvar to make the thread sleep, and brings the code more in line with the other branches... 729496 12 165992 mark.toller 2012-09-27 07:02:53 -0700 Created attachment 165992 Patch 729499 13 webkit.review.bot 2012-09-27 07:05:53 -0700 Attachment 165992 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Fa..." exit_code: 1 Source/WTF/wtf/FastMalloc.cpp:2553: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2555: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2558: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2559: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2562: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2566: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2567: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2568: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2569: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2571: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WTF/wtf/FastMalloc.cpp:2573: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 11 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 729636 14 ap 2012-09-27 09:54:12 -0700 Thank you. I've been out of touch with this code for too long to easily review, CC'ing some potential reviewers. Please upload a new patch with style bot comments addressed, this is quite hard to read in review patch view due to those comments. 729697 15 165992 ggaren 2012-09-27 10:51:40 -0700 Comment on attachment 165992 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165992&action=review >> Source/WTF/wtf/FastMalloc.cpp:2555 >> + // m_scavengeThreadActive protected by pageheap_lock. > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] m_scavengeThreadActive is an atomic variable. Why do you need to lock across assignment? > Source/WTF/wtf/FastMalloc.cpp:2564 > pthread_mutex_unlock(&m_scavengeMutex); I believe this unlock is wrong, as per the pthread_cond_mutex man page: The pthread_cond_wait() function atomically unlocks the mutex argument and waits on the cond argument. Before returning control to the calling function, pthread_cond_wait() re-acquires the mutex. 730329 16 mark.toller 2012-09-28 00:59:24 -0700 (In reply to comment #15) > m_scavengeThreadActive is an atomic variable. Why do you need to lock across assignment? I think you're right. However, we should lock pageheap_lock before calling 'shouldScavenge()', as we only want to scavenge when really required, so it's better to wait for any current memory operation to complete before checking. In this case, we may as well set m_scavengeThreadActive = false while still holding the lock. Do we need to re-lock before setting m_scavengeThreadActive = true ? No. In fact, it's probably better to do so outside of the lock, as this could prevent an unnecessary signal of the condvar (if we waited for a current delete op holding the spinlock to complete, which calls signalScavenger()). > > > Source/WTF/wtf/FastMalloc.cpp:2564 > > pthread_mutex_unlock(&m_scavengeMutex); > > I believe this unlock is wrong, as per the pthread_cond_mutex man page: > > > The pthread_cond_wait() function atomically unlocks the mutex argument > and waits on the cond argument. Before returning control to the calling > function, pthread_cond_wait() re-acquires the mutex. If we don't unlock the mutex (type is PTHREAD_MUTEX_NORMAL) which we now hold after exiting the pthread_cond_wait, then next time around the loop we will deadlock: If the mutex type is PTHREAD_MUTEX_NORMAL, deadlock detection is not provided. Attempting to relock the mutex causes deadlock. 730330 17 166162 mark.toller 2012-09-28 00:59:55 -0700 Created attachment 166162 Patch 739457 18 166162 ggaren 2012-10-10 18:08:04 -0700 Comment on attachment 166162 Patch r=me 740016 19 166162 webkit.review.bot 2012-10-11 08:53:58 -0700 Comment on attachment 166162 Patch Clearing flags on attachment: 166162 Committed r131066: <http://trac.webkit.org/changeset/131066> 740017 20 webkit.review.bot 2012-10-11 08:54:02 -0700 All reviewed patches have been landed. Closing bug. 165565 2012-09-25 02:41:51 -0700 2012-09-26 06:03:12 -0700 Patch bug-97539-20120925104134.patch text/plain 1977 mark.toller SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAxMjk0ODApCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDEyLTA5LTI1ICBNYXJrIFRvbGxlciAgPG1hcmsu dG9sbGVyQHNhbXN1bmcuY29tPgorCisgICAgICAgIFJlbW92ZWQgaW5jb3JyZWN0IHB0aHJlYWRf bXV0ZXhfdHJ5bG9jayBjb2RlIGluIGFuIEFTU0VSVCBpbiBUQ01hbGxvY19QYWdlSGVhcDo6c2ln bmFsU2NhdmVuZ2VyLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD05NzUzOQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFRoZSBjb2RlIHdhcyBuZXZlciBjb21waWxlZCBpbiwgYW5kIHdhcyBmdW5jdGlvbmFs bHkgYnJva2VuLiBJbiBvcmRlciB0byByZWR1Y2UgbXV0ZXggbG9jay91bmxvY2sgb24gbWFueQor ICAgICAgICBkZWxldGUgb3BlcmF0aW9ucywgdGhlIGNvZGUgaXMgc2ltcGx5IHJlbW92ZWQuCisK KyAgICAgICAgKiB3dGYvRmFzdE1hbGxvYy5jcHA6CisgICAgICAgIChXVEY6OlRDTWFsbG9jX1Bh Z2VIZWFwOjpzaWduYWxTY2F2ZW5nZXIpOgorCiAyMDEyLTA5LTIxICBJbHlhIFRpa2hvbm92c2t5 ICA8bG9pc2xvQGNocm9taXVtLm9yZz4KIAogICAgICAgICBXZWIgSW5zcGVjdG9yOiBleHRyYWN0 IFZlY3RvciBpbnN0cnVtZW50YXRpb24gZnJvbSBjb3JlIE5NSSBjb2RlIGludG8gTWVtb3J5SW5z dHJ1bWVudGF0aW9uVmVjdG9yLmggaGVhZGVyLgpJbmRleDogU291cmNlL1dURi93dGYvRmFzdE1h bGxvYy5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dURi93dGYvRmFzdE1hbGxvYy5jcHAJKHJl dmlzaW9uIDEyOTQ3OSkKKysrIFNvdXJjZS9XVEYvd3RmL0Zhc3RNYWxsb2MuY3BwCSh3b3JraW5n IGNvcHkpCkBAIC0xNjI3LDggKzE2MjcsMTIgQEAgdm9pZCogVENNYWxsb2NfUGFnZUhlYXA6OnJ1 blNjYXZlbmdlclRocgogCiBBTFdBWVNfSU5MSU5FIHZvaWQgVENNYWxsb2NfUGFnZUhlYXA6OnNp Z25hbFNjYXZlbmdlcigpCiB7Ci0gICAgLy8gbV9zY2F2ZW5nZU11dGV4IHNob3VsZCBiZSBoZWxk IGJlZm9yZSBhY2Nlc3NpbmcgbV9zY2F2ZW5nZVRocmVhZEFjdGl2ZS4KLSAgICBBU1NFUlQocHRo cmVhZF9tdXRleF90cnlsb2NrKG1fc2NhdmVuZ2VNdXRleCkpOworICAgIC8vIEluIGFuIGlkZWFs IHNjZW5hcmlvLCB3ZSB3b3VsZCBsb2NrIG1fc2NhdmVuZ2VNdXRleCBiZWZvcmUgYWNjZXNzaW5n IG1fc2NhdmVuZ2VUaHJlYWRBY3RpdmUuIEhvd2V2ZXIsIHRoaXMKKyAgICAvLyB3aWxsIGNhdXNl IHVzIHRvIHBlcmZvcm0gYSBsb2NrL3VubG9jayBvbiB0aGUgbXV0ZXggZm9yIG1hbnkgZGVsZXRl IG9wZXJhdGlvbnMuIFRoZSB3b3JzdCBjYXNlIHNjZW5hcmlvIGZvcgorICAgIC8vICpub3QqIGxv Y2tpbmcgdGhlIG11dGV4IGlzIHRoYXQgd2UgZWl0aGVyOgorICAgIC8vIDEpIHNpZ25hbCB0aGUg Y29uZHZhciBtb3JlIG9mdGVuIHRoYW4gcmVxdWlyZWQgd2hlbiB3ZSBnZXQgc2V2ZXJhbCBkZWxl dGVzIGJlZm9yZSB0aGUgbV9zY2F2ZW5nZVRocmVhZEFjdGl2ZQorICAgIC8vIGlzIHNldCB0byB0 cnVlLCBvcjoKKyAgICAvLyAyKSBXZSBkb24ndCBzaWduYWwgdGhlIGNvbmR2YXIgYXMgZWFybHkg YXMgcG9zc2libGUsIGFuZCB3YWl0IHRpbGwgdGhlIG5leHQgZGVsZXRlLgogICAgIGlmICghbV9z Y2F2ZW5nZVRocmVhZEFjdGl2ZSAmJiBzaG91bGRTY2F2ZW5nZSgpKQogICAgICAgICBwdGhyZWFk X2NvbmRfc2lnbmFsKCZtX3NjYXZlbmdlQ29uZGl0aW9uKTsKIH0K 165777 2012-09-26 06:03:18 -0700 2012-09-27 07:02:46 -0700 Patch bug-97539-20120926140259.patch text/plain 2435 mark.toller SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAxMjk2MjQpCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBACisyMDEyLTA5LTI1ICBNYXJrIFRvbGxlciAgPG1hcmsu dG9sbGVyQHNhbXN1bmcuY29tPgorCisgICAgICAgIFJlbW92ZWQgaW5jb3JyZWN0IHB0aHJlYWRf bXV0ZXhfdHJ5bG9jayBjb2RlIGluIGFuIEFTU0VSVCBpbiBUQ01hbGxvY19QYWdlSGVhcDo6c2ln bmFsU2NhdmVuZ2VyLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD05NzUzOQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFRoZSBjb2RlIHdhcyBuZXZlciBjb21waWxlZCBpbiwgYW5kIHdhcyBmdW5jdGlvbmFs bHkgYnJva2VuLiBJbiBvcmRlciB0byByZWR1Y2UgbXV0ZXggbG9jay91bmxvY2sgb24gbWFueQor ICAgICAgICBkZWxldGUgb3BlcmF0aW9ucywgdGhlIGNvZGUgaXMgc2ltcGx5IHJlbW92ZWQuCisg ICAgICAgIElmIHRoZSAoZml4ZWQpIGNvZGUgd2FzIGNvbXBpbGVkIGluLCBpdCB3b3VsZCBlaXRo ZXI6CisgICAgICAgICAgICBhKSBjYXVzZSBhIGRlYWRsb2NrIGlmIHRoZSBtdXRleCBpcyBsb2Nr ZWQgc3VjY2Vzc2Z1bGx5LCBhcyBpdCBpcyBub3QgdW5sb2NrZWQgYWZ0ZXJ3YXJkcy4KKyAgICAg ICAgICAgIGIpIGNhdXNlIHRoZSBBU1NFUlQgdG8gZmlyZSBpZiB0aGUgbXV0ZXggaXMgbm90IHRh a2VuICh3aGljaCBjYW4gYW5kIGRvZXMgaGFwcGVuKS4KKyAgICAgICAgQWRkaW5nIGEgcHRocmVh ZF9tdXRleF9sb2NrL3B0aHJlYWRfbXV0ZXhfdW5sb2NrIHBhaXIgYXJvdW5kIHRoZSBjb2RlIGlu dHJvZHVjZXMgbWFueSB1bm5lY2Nlc3NhcnkgY2FsbHMsIGFzCisgICAgICAgIHRoZXJlIGlzIG5v ICpuZWVkKiB0byBsb2NrIHRoZSBtdXRleCB3aGVuIGNoZWNraW5nIG1fc2NhdmVuZ2VUaHJlYWRB Y3RpdmUuCisKKyAgICAgICAgKiB3dGYvRmFzdE1hbGxvYy5jcHA6CisgICAgICAgIChXVEY6OlRD TWFsbG9jX1BhZ2VIZWFwOjpzaWduYWxTY2F2ZW5nZXIpOgorCiAyMDEyLTA5LTI2ICBZdXJ5IFNl bWlraGF0c2t5ICA8eXVyeXNAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6 IGNvbXBhcmUgb2JqZWN0cyBjb3VudGVkIGJ5IHRoZSBtZW1vcnkgaW5zdHJ1bWVudGF0aW9uIHdp dGggdGhvc2UgYWxsb2NhdGVkIGluIHRoZSBoZWFwCkluZGV4OiBTb3VyY2UvV1RGL3d0Zi9GYXN0 TWFsbG9jLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV1RGL3d0Zi9GYXN0TWFsbG9jLmNwcAko cmV2aXNpb24gMTI5NjI0KQorKysgU291cmNlL1dURi93dGYvRmFzdE1hbGxvYy5jcHAJKHdvcmtp bmcgY29weSkKQEAgLTE2MjcsOCArMTYyNywxMiBAQCB2b2lkKiBUQ01hbGxvY19QYWdlSGVhcDo6 cnVuU2NhdmVuZ2VyVGhyCiAKIEFMV0FZU19JTkxJTkUgdm9pZCBUQ01hbGxvY19QYWdlSGVhcDo6 c2lnbmFsU2NhdmVuZ2VyKCkKIHsKLSAgICAvLyBtX3NjYXZlbmdlTXV0ZXggc2hvdWxkIGJlIGhl bGQgYmVmb3JlIGFjY2Vzc2luZyBtX3NjYXZlbmdlVGhyZWFkQWN0aXZlLgotICAgIEFTU0VSVChw dGhyZWFkX211dGV4X3RyeWxvY2sobV9zY2F2ZW5nZU11dGV4KSk7CisgICAgLy8gSW4gYW4gaWRl YWwgc2NlbmFyaW8sIHdlIHdvdWxkIGxvY2sgbV9zY2F2ZW5nZU11dGV4IGJlZm9yZSBhY2Nlc3Np bmcgbV9zY2F2ZW5nZVRocmVhZEFjdGl2ZS4gSG93ZXZlciwgdGhpcworICAgIC8vIHdpbGwgY2F1 c2UgdXMgdG8gcGVyZm9ybSBhIGxvY2svdW5sb2NrIG9uIHRoZSBtdXRleCBmb3IgbWFueSBkZWxl dGUgb3BlcmF0aW9ucy4gVGhlIHdvcnN0IGNhc2Ugc2NlbmFyaW8gZm9yCisgICAgLy8gKm5vdCog bG9ja2luZyB0aGUgbXV0ZXggaXMgdGhhdCB3ZSBlaXRoZXI6CisgICAgLy8gMSkgc2lnbmFsIHRo ZSBjb25kdmFyIG1vcmUgb2Z0ZW4gdGhhbiByZXF1aXJlZCB3aGVuIHdlIGdldCBzZXZlcmFsIGRl bGV0ZXMgYmVmb3JlIHRoZSBtX3NjYXZlbmdlVGhyZWFkQWN0aXZlCisgICAgLy8gaXMgc2V0IHRv IHRydWUsIG9yOgorICAgIC8vIDIpIFdlIGRvbid0IHNpZ25hbCB0aGUgY29uZHZhciBhcyBlYXJs eSBhcyBwb3NzaWJsZSwgYW5kIHdhaXQgdGlsbCB0aGUgbmV4dCBkZWxldGUuCiAgICAgaWYgKCFt X3NjYXZlbmdlVGhyZWFkQWN0aXZlICYmIHNob3VsZFNjYXZlbmdlKCkpCiAgICAgICAgIHB0aHJl YWRfY29uZF9zaWduYWwoJm1fc2NhdmVuZ2VDb25kaXRpb24pOwogfQo= 165992 2012-09-27 07:02:53 -0700 2012-09-28 00:59:48 -0700 Patch bug-97539-20120927150233.patch text/plain 3135 mark.toller SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAxMjk3MjgpCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA5LTI3ICBNYXJrIFRvbGxlciAgPG1hcmsu dG9sbGVyQHNhbXN1bmcuY29tPgorCisgICAgICAgIFJlbW92ZWQgaW5jb3JyZWN0IHB0aHJlYWRf bXV0ZXhfdHJ5bG9jayBjb2RlIGluIGFuIEFTU0VSVCBpbiBUQ01hbGxvY19QYWdlSGVhcDo6c2ln bmFsU2NhdmVuZ2VyLiBUaGlzIGJyYW5jaAorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD05NzUzOQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIFRoZSBjb2RlIHdhcyBuZXZlciBjb21waWxlZCBpbiwgYW5kIHdh cyBmdW5jdGlvbmFsbHkgYnJva2VuLiBJbiBvcmRlciB0byByZWR1Y2UgbXV0ZXggbG9jay91bmxv Y2sgb24gbWFueQorICAgICAgICBkZWxldGUgb3BlcmF0aW9ucyBieSBpbnRyZHVjaW5nIGEgcHRo cmVhZF9tdXRleF9sb2NrL3B0aHJlYWRfbXV0ZXhfdW5sb2NrIHBhaXIgYXJvdW5kIHRoZSBjb2Rl LCB0aGUgcHJvdGVjdGlvbiAKKyAgICAgICAgZm9yIG1fc2NhdmVuZ2VUaHJlYWRBY3RpdmUgaXMg cmVwbGFjZWQgYnkgdGhlIHBhZ2VoZWFwX2xvY2sgc3BpbmxvY2suCisKKyAgICAgICAgKiB3dGYv RmFzdE1hbGxvYy5jcHA6CisgICAgICAgIChXVEY6OlRDTWFsbG9jX1BhZ2VIZWFwOjpzaWduYWxT Y2F2ZW5nZXIpOgorCiAyMDEyLTA5LTI2ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUu Y29tPgogCiAgICAgICAgIFVwZGF0ZSBDb21wbGV4VGV4dENvbnRyb2xsZXIgZm9yIDggYml0IFRl eHRSdW4gY2hhbmdlcwpJbmRleDogU291cmNlL1dURi93dGYvRmFzdE1hbGxvYy5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dURi93dGYvRmFzdE1hbGxvYy5jcHAJKHJldmlzaW9uIDEyOTc2MykK KysrIFNvdXJjZS9XVEYvd3RmL0Zhc3RNYWxsb2MuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNjI3 LDggKzE2MjcsMTAgQEAgdm9pZCogVENNYWxsb2NfUGFnZUhlYXA6OnJ1blNjYXZlbmdlclRocgog CiBBTFdBWVNfSU5MSU5FIHZvaWQgVENNYWxsb2NfUGFnZUhlYXA6OnNpZ25hbFNjYXZlbmdlcigp CiB7Ci0gICAgLy8gbV9zY2F2ZW5nZU11dGV4IHNob3VsZCBiZSBoZWxkIGJlZm9yZSBhY2Nlc3Np bmcgbV9zY2F2ZW5nZVRocmVhZEFjdGl2ZS4KLSAgICBBU1NFUlQocHRocmVhZF9tdXRleF90cnls b2NrKG1fc2NhdmVuZ2VNdXRleCkpOworICAgIC8vIEFjY2VzcyB0byBtX3NjYXZlbmdlVGhyZWFk QWN0aXZlIGlzIHByb3RlY3RlZCBieSB0aGUgcGFnZWhlYXBfbG9jayBzcGlubG9jay4gVGhlIGNh bGxlciBtdXN0IGVuc3VyZSB0aGlzIGlzCisgICAgLy8gdGFrZW4gcHJpb3IgdG8gY2FsbGluZyB0 aGlzIG1ldGhvZC4gSWYgdGhlIHNjYXZlbmdlciB0aHJlYWQgaXMgc2xlZXBpbmcgYW5kIHNob3Vs ZFNjYXZlbmdlKCkgaW5kaWNhdGVzIHRoZXJlCisgICAgLy8gaXMgbWVtb3J5IHRvIGZyZWUgdGhl IHNjYXZlbmdlciB0aHJlYWQgaXMgc2lnbmFsbGVkIHRvIHN0YXJ0LgorICAgIEFTU0VSVChwYWdl aGVhcF9sb2NrLklzSGVsZCgpKTsKICAgICBpZiAoIW1fc2NhdmVuZ2VUaHJlYWRBY3RpdmUgJiYg c2hvdWxkU2NhdmVuZ2UoKSkKICAgICAgICAgcHRocmVhZF9jb25kX3NpZ25hbCgmbV9zY2F2ZW5n ZUNvbmRpdGlvbik7CiB9CkBAIC0yNTQ4LDE1ICsyNTUwLDI5IEBAIHZvaWQgVENNYWxsb2NfUGFn ZUhlYXA6OnNjYXZlbmdlclRocmVhZCgKICNlbmRpZgogCiAgIHdoaWxlICgxKSB7CisgICAgICBw YWdlaGVhcF9sb2NrLkxvY2soKTsKICAgICAgIGlmICghc2hvdWxkU2NhdmVuZ2UoKSkgewotICAg ICAgICAgIHB0aHJlYWRfbXV0ZXhfbG9jaygmbV9zY2F2ZW5nZU11dGV4KTsKKyAgICAgICAgICAv LyBtX3NjYXZlbmdlVGhyZWFkQWN0aXZlIHByb3RlY3RlZCBieSBwYWdlaGVhcF9sb2NrLgogICAg ICAgICAgIG1fc2NhdmVuZ2VUaHJlYWRBY3RpdmUgPSBmYWxzZTsKKworICAgICAgICAgIC8vIFdl IG5lZWQgdG8gdW5sb2NrIG5vdywgYXMgdGhpcyB0aHJlYWQgd2lsbCBibG9jayBvbiB0aGUgY29u ZHZhciB1bnRpbCBzY2F2ZW5naW5nIGlzIHJlcXVpcmVkLgorICAgICAgICAgIHBhZ2VoZWFwX2xv Y2suVW5sb2NrKCk7CisKICAgICAgICAgICAvLyBCbG9jayB1bnRpbCB0aGVyZSBhcmUgZW5vdWdo IGZyZWUgY29tbWl0dGVkIHBhZ2VzIHRvIHJlbGVhc2UgYmFjayB0byB0aGUgc3lzdGVtLgorICAg ICAgICAgIHB0aHJlYWRfbXV0ZXhfbG9jaygmbV9zY2F2ZW5nZU11dGV4KTsKICAgICAgICAgICBw dGhyZWFkX2NvbmRfd2FpdCgmbV9zY2F2ZW5nZUNvbmRpdGlvbiwgJm1fc2NhdmVuZ2VNdXRleCk7 Ci0gICAgICAgICAgbV9zY2F2ZW5nZVRocmVhZEFjdGl2ZSA9IHRydWU7CiAgICAgICAgICAgcHRo cmVhZF9tdXRleF91bmxvY2soJm1fc2NhdmVuZ2VNdXRleCk7CisKKyAgICAgICAgICAvLyBSZS1s b2NrIHRoZSBwYWdlaGVhcF9sb2NrIHRvIHRvZ2dsZSB0aGUgbV9zY2F2ZW5nZVRocmVhZEFjdGl2 ZSBmbGFnLCBhbmQgc28gdGhlIHVubG9jayBvdXRzaWRlIHRoZSBsb29wCisgICAgICAgICAgLy8g aXMgb2suCisgICAgICAgICAgcGFnZWhlYXBfbG9jay5Mb2NrKCk7CisgICAgICAgICAgbV9zY2F2 ZW5nZVRocmVhZEFjdGl2ZSA9IHRydWU7CiAgICAgICB9CisgICAgICBwYWdlaGVhcF9sb2NrLlVu bG9jaygpOworCisgICAgICAvLyBXYWl0IGZvciBhIHdoaWxlIHRvIGNhbGN1bGF0ZSBob3cgbXVj aCBtZW1vcnkgcmVtYWlucyB1bnVzZWQgZHVyaW5nIHRoaXMgcGF1c2UuCiAgICAgICBzbGVlcChr U2NhdmVuZ2VEZWxheUluU2Vjb25kcyk7CisKICAgICAgIHsKICAgICAgICAgICBTcGluTG9ja0hv bGRlciBoKCZwYWdlaGVhcF9sb2NrKTsKICAgICAgICAgICBwYWdlaGVhcC0+c2NhdmVuZ2UoKTsK 166162 2012-09-28 00:59:55 -0700 2012-10-11 08:53:58 -0700 Patch bug-97539-20120928085934.patch text/plain 4176 mark.toller SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAxMjk3MjgpCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEyLTA5LTI3ICBNYXJrIFRvbGxlciAgPG1hcmsu dG9sbGVyQHNhbXN1bmcuY29tPgorCisgICAgICAgIFJlbW92ZWQgaW5jb3JyZWN0IHB0aHJlYWRf bXV0ZXhfdHJ5bG9jayBjb2RlIGluIGFuIEFTU0VSVCBpbiBUQ01hbGxvY19QYWdlSGVhcDo6c2ln bmFsU2NhdmVuZ2VyLiBUaGlzIAorICAgICAgICBicmFuY2ggaXMgdXNlZCBieSB0aGUgV2Via2l0 IEdUSyBjb2RlLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD05NzUzOQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFRoZSBjb2RlIHdhcyBuZXZlciBjb21waWxlZCBpbiwgYW5kIHdhcyBmdW5jdGlvbmFsbHkg YnJva2VuLiBUaGVyZSBpcyBubyBuZWVkIGZvciBsb2NraW5nIGFyb3VuZCB0aGUgCisgICAgICAg IG1fc2NhdmVuZ2VUaHJlYWRBY3RpdmUgZmxhZywgaG93ZXZlciwgd2Ugc2hvdWxkIGxvY2sgcGFn ZWhlYXBfbG9jayBiZWZvcmUgY2FsbGluZyAnc2hvdWxkU2NhdmVuZ2UoKScsIGFzIHdlCisgICAg ICAgIG9ubHkgd2FudCB0byBzY2F2ZW5nZSB3aGVuIHJlYWxseSByZXF1aXJlZCwgc28gaXQncyBi ZXR0ZXIgdG8gd2FpdCBmb3IgYW55IGN1cnJlbnQgbWVtb3J5IG9wZXJhdGlvbiB0byAKKyAgICAg ICAgY29tcGxldGUgYmVmb3JlIGNoZWNraW5nLiAKKworICAgICAgICAqIHd0Zi9GYXN0TWFsbG9j LmNwcDoKKyAgICAgICAgKFdURjo6VENNYWxsb2NfUGFnZUhlYXA6OnNpZ25hbFNjYXZlbmdlcik6 CisKIDIwMTItMDktMjYgIE1pY2hhZWwgU2Fib2ZmICA8bXNhYm9mZkBhcHBsZS5jb20+CiAKICAg ICAgICAgVXBkYXRlIENvbXBsZXhUZXh0Q29udHJvbGxlciBmb3IgOCBiaXQgVGV4dFJ1biBjaGFu Z2VzCkluZGV4OiBTb3VyY2UvV1RGL3d0Zi9GYXN0TWFsbG9jLmNwcAo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBT b3VyY2UvV1RGL3d0Zi9GYXN0TWFsbG9jLmNwcAkocmV2aXNpb24gMTI5NzYzKQorKysgU291cmNl L1dURi93dGYvRmFzdE1hbGxvYy5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE2MjcsOCArMTYyNywx MSBAQCB2b2lkKiBUQ01hbGxvY19QYWdlSGVhcDo6cnVuU2NhdmVuZ2VyVGhyCiAKIEFMV0FZU19J TkxJTkUgdm9pZCBUQ01hbGxvY19QYWdlSGVhcDo6c2lnbmFsU2NhdmVuZ2VyKCkKIHsKLSAgICAv LyBtX3NjYXZlbmdlTXV0ZXggc2hvdWxkIGJlIGhlbGQgYmVmb3JlIGFjY2Vzc2luZyBtX3NjYXZl bmdlVGhyZWFkQWN0aXZlLgotICAgIEFTU0VSVChwdGhyZWFkX211dGV4X3RyeWxvY2sobV9zY2F2 ZW5nZU11dGV4KSk7CisgICAgLy8gc2hvdWxkU2NhdmVuZ2UoKSBzaG91bGQgYmUgY2FsbGVkIG9u bHkgd2hlbiB0aGUgcGFnZWhlYXBfbG9jayBzcGlubG9jayBpcyBoZWxkLCBhZGRpdGlvbmFsbHks IAorICAgIC8vIG1fc2NhdmVuZ2VUaHJlYWRBY3RpdmUgaXMgb25seSBzZXQgdG8gZmFsc2Ugd2hp bHN0IHBhZ2VoZWFwX2xvY2sgaXMgaGVsZC4gVGhlIGNhbGxlciBtdXN0IGVuc3VyZSB0aGlzIGlz CisgICAgLy8gdGFrZW4gcHJpb3IgdG8gY2FsbGluZyB0aGlzIG1ldGhvZC4gSWYgdGhlIHNjYXZl bmdlciB0aHJlYWQgaXMgc2xlZXBpbmcgYW5kIHNob3VsZFNjYXZlbmdlKCkgaW5kaWNhdGVzIHRo ZXJlCisgICAgLy8gaXMgbWVtb3J5IHRvIGZyZWUgdGhlIHNjYXZlbmdlciB0aHJlYWQgaXMgc2ln bmFsbGVkIHRvIHN0YXJ0LgorICAgIEFTU0VSVChwYWdlaGVhcF9sb2NrLklzSGVsZCgpKTsKICAg ICBpZiAoIW1fc2NhdmVuZ2VUaHJlYWRBY3RpdmUgJiYgc2hvdWxkU2NhdmVuZ2UoKSkKICAgICAg ICAgcHRocmVhZF9jb25kX3NpZ25hbCgmbV9zY2F2ZW5nZUNvbmRpdGlvbik7CiB9CkBAIC0yNTQ0 LDI0ICsyNTQ3LDM4IEBAIEFMV0FZU19JTkxJTkUgdm9pZCBUQ01hbGxvY19QYWdlSGVhcDo6c2kK IHZvaWQgVENNYWxsb2NfUGFnZUhlYXA6OnNjYXZlbmdlclRocmVhZCgpCiB7CiAjaWYgSEFWRShQ VEhSRUFEX1NFVE5BTUVfTlApCi0gIHB0aHJlYWRfc2V0bmFtZV9ucCgiSmF2YVNjcmlwdENvcmU6 IEZhc3RNYWxsb2Mgc2NhdmVuZ2VyIik7CisgICAgcHRocmVhZF9zZXRuYW1lX25wKCJKYXZhU2Ny aXB0Q29yZTogRmFzdE1hbGxvYyBzY2F2ZW5nZXIiKTsKICNlbmRpZgogCi0gIHdoaWxlICgxKSB7 Ci0gICAgICBpZiAoIXNob3VsZFNjYXZlbmdlKCkpIHsKLSAgICAgICAgICBwdGhyZWFkX211dGV4 X2xvY2soJm1fc2NhdmVuZ2VNdXRleCk7Ci0gICAgICAgICAgbV9zY2F2ZW5nZVRocmVhZEFjdGl2 ZSA9IGZhbHNlOwotICAgICAgICAgIC8vIEJsb2NrIHVudGlsIHRoZXJlIGFyZSBlbm91Z2ggZnJl ZSBjb21taXR0ZWQgcGFnZXMgdG8gcmVsZWFzZSBiYWNrIHRvIHRoZSBzeXN0ZW0uCi0gICAgICAg ICAgcHRocmVhZF9jb25kX3dhaXQoJm1fc2NhdmVuZ2VDb25kaXRpb24sICZtX3NjYXZlbmdlTXV0 ZXgpOwotICAgICAgICAgIG1fc2NhdmVuZ2VUaHJlYWRBY3RpdmUgPSB0cnVlOwotICAgICAgICAg IHB0aHJlYWRfbXV0ZXhfdW5sb2NrKCZtX3NjYXZlbmdlTXV0ZXgpOwotICAgICAgfQotICAgICAg c2xlZXAoa1NjYXZlbmdlRGVsYXlJblNlY29uZHMpOwotICAgICAgewotICAgICAgICAgIFNwaW5M b2NrSG9sZGVyIGgoJnBhZ2VoZWFwX2xvY2spOwotICAgICAgICAgIHBhZ2VoZWFwLT5zY2F2ZW5n ZSgpOwotICAgICAgfQotICB9CisgICAgd2hpbGUgKDEpIHsKKyAgICAgICAgcGFnZWhlYXBfbG9j ay5Mb2NrKCk7CisgICAgICAgIGlmICghc2hvdWxkU2NhdmVuZ2UoKSkgeworICAgICAgICAgICAg Ly8gU2V0IHRvIGZhbHNlIHNvIHRoYXQgc2lnbmFsU2NhdmVuZ2VyKCkgd2lsbCBjaGVjayB3aGV0 aGVyIHdlIG5lZWQgdG8gYmUgc2lnYW5sbGVkLgorICAgICAgICAgICAgbV9zY2F2ZW5nZVRocmVh ZEFjdGl2ZSA9IGZhbHNlOworCisgICAgICAgICAgICAvLyBXZSBuZWVkIHRvIHVubG9jayBub3cs IGFzIHRoaXMgdGhyZWFkIHdpbGwgYmxvY2sgb24gdGhlIGNvbmR2YXIgdW50aWwgc2NhdmVuZ2lu ZyBpcyByZXF1aXJlZC4KKyAgICAgICAgICAgIHBhZ2VoZWFwX2xvY2suVW5sb2NrKCk7CisKKyAg ICAgICAgICAgIC8vIEJsb2NrIHVudGlsIHRoZXJlIGFyZSBlbm91Z2ggZnJlZSBjb21taXR0ZWQg cGFnZXMgdG8gcmVsZWFzZSBiYWNrIHRvIHRoZSBzeXN0ZW0uCisgICAgICAgICAgICBwdGhyZWFk X211dGV4X2xvY2soJm1fc2NhdmVuZ2VNdXRleCk7CisgICAgICAgICAgICBwdGhyZWFkX2NvbmRf d2FpdCgmbV9zY2F2ZW5nZUNvbmRpdGlvbiwgJm1fc2NhdmVuZ2VNdXRleCk7CisgICAgICAgICAg ICAvLyBBZnRlciBleGl0aW5nIHRoZSBwdGhyZWFkX2NvbmRfd2FpdCwgd2UgaG9sZCB0aGUgbG9j ayBvbiBtX3NjYXZlbmdlTXV0ZXguIFVubG9jayBpdCB0byBwcmV2ZW50CisgICAgICAgICAgICAv LyBkZWFkbG9jayBuZXh0IHRpbWUgcm91bmQgdGhlIGxvb3AuCisgICAgICAgICAgICBwdGhyZWFk X211dGV4X3VubG9jaygmbV9zY2F2ZW5nZU11dGV4KTsKKworICAgICAgICAgICAgLy8gU2V0IHRv IHRydWUgdG8gcHJldmVudCB1bm5lY2Vzc2FyeSBzaWduYWxsaW5nIG9mIHRoZSBjb25kdmFyLgor ICAgICAgICAgICAgbV9zY2F2ZW5nZVRocmVhZEFjdGl2ZSA9IHRydWU7CisgICAgICAgIH0gZWxz ZQorICAgICAgICAgICAgcGFnZWhlYXBfbG9jay5VbmxvY2soKTsKKworICAgICAgICAvLyBXYWl0 IGZvciBhIHdoaWxlIHRvIGNhbGN1bGF0ZSBob3cgbXVjaCBtZW1vcnkgcmVtYWlucyB1bnVzZWQg ZHVyaW5nIHRoaXMgcGF1c2UuCisgICAgICAgIHNsZWVwKGtTY2F2ZW5nZURlbGF5SW5TZWNvbmRz KTsKKworICAgICAgICB7CisgICAgICAgICAgICBTcGluTG9ja0hvbGRlciBoKCZwYWdlaGVhcF9s b2NrKTsKKyAgICAgICAgICAgIHBhZ2VoZWFwLT5zY2F2ZW5nZSgpOworICAgICAgICB9CisgICAg fQogfQogCiAjZW5kaWYK