162702 2016-09-28 14:04:01 -0700 Fix race condition in StringView's UnderlyingString lifecycle management. 2016-09-29 17:01:57 -0700 1 1 1 Unclassified WebKit Web Template Framework WebKit Local Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 160384 1 mark.lam mark.lam benjamin cdumez cmarcelo commit-queue darin dbates ddkilzer ggaren oldest_to_newest 1234544 0 mark.lam 2016-09-28 14:04:01 -0700 There 2 relevant functions at play: void StringView::setUnderlyingString(const StringImpl* string) { UnderlyingString* underlyingString; if (!string) underlyingString = nullptr; else { std::lock_guard<StaticLock> lock(underlyingStringsMutex); auto result = underlyingStrings().add(string, nullptr); if (result.isNewEntry) result.iterator->value = new UnderlyingString(*string); else ++result.iterator->value->refCount; underlyingString = result.iterator->value; // Point P2. } adoptUnderlyingString(underlyingString); // Point P5. } ... and ... void StringView::adoptUnderlyingString(UnderlyingString* underlyingString) { if (m_underlyingString) { // Point P0. if (!--m_underlyingString->refCount) { if (m_underlyingString->isValid) { // Point P1. std::lock_guard<StaticLock> lock(underlyingStringsMutex); underlyingStrings().remove(&m_underlyingString->string); // Point P3. } delete m_underlyingString; // Point P4. } } m_underlyingString = underlyingString; } Imagine the following scenario: 1. Thread T1 has been using an UnderlyingString U1, and is now done with it. T1 runs up to point P1 in adoptUnderlyingString(), and is blocked waiting for the underlyingStringsMutex (which is currently being held by Thread T2). 2. Context switch to Thread T2. T2 wants to use UnderlyingString U1, and runs up to point P2 in setUnderlyingString() and releases the underlyingStringsMutex. Note: T2 thinks it has successfully refCounted U1, and therefore U1 is safe to use. 3. Context switch to Thread T1. T1 acquires the underlyingStringsMutex, and proceeds to remove it from the underlyingStrings() map (see Point P3). It thinks it has successfully done so and proceeds to delete U1 (see Point P4). 4. Context switch to Thread T2. T2 proceeds to use U1 (see Point P5 in setUnderlyingString()). Note: U1 has already been freed. This is a use after free. The fix is to acquire the underlyingStringsMutex at Point P0 in adoptUnderlyingString() instead of after P1. This ensures that the decrementing of the UnderlyingString refCount and its removal from the underlyingStrings() map is done as an atomic unit. Note: If you look in StringView.cpp, you see another setUnderlyingString() which takes a StringView otherString. This version of setUnderlyingString() can only be called from within the same thread that created the other StringView. As a result, here, we are guaranteed that the UnderlyingString refCount is never zero, and there's no other threat of another thread trying to delete the UnderlyingString while we adopt it. Hence, we don't need to acquire the underlyingStringsMutex here. 1234545 1 290119 mark.lam 2016-09-28 14:11:18 -0700 Created attachment 290119 proposed patch. 1234552 2 290119 ggaren 2016-09-28 14:29:17 -0700 Comment on attachment 290119 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=290119&action=review r=me > Source/WTF/wtf/text/StringView.cpp:272 > + // StringView instances are not passed across threads. Hence, since we have a > + // reference to otherString, we are guaranteed that its underlyingString refCount > + // will remain non-zero for us to increment. We don't need to grab the > + // underlyingStringsMutex. It's a general principle that any caller that passes a const& object should not pass garbage memory, so I don't think this comment adds anything -- and it might harm readability by suggesting that anything more special than that is happening here. 1234558 3 mark.lam 2016-09-28 14:33:53 -0700 Thanks for the review. I removed the comment in setUnderlyingString(). Landed in r206552: <http://trac.webkit.org/r206552>. 1234923 4 290119 darin 2016-09-29 12:43:05 -0700 Comment on attachment 290119 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=290119&action=review > Source/WTF/wtf/text/StringView.cpp:240 > + std::lock_guard<StaticLock> lock(underlyingStringsMutex); > if (!--m_underlyingString->refCount) { Can we make UnderlyingString::refCount an atomic instead? I am concerned that the locking will slow things down too much. 1234927 5 darin 2016-09-29 12:55:40 -0700 Not that the performance here is critical, but even in debug builds this is hot enough that it might be nice to find a way to make it faster. 1234931 6 darin 2016-09-29 12:59:21 -0700 I also suspect that in the original case where this was failing, there is actually a thread safety problem in the client code, and the root cause is not the lack of thread safety in StringView itself. After all, the reference counts of the strings themselves are not guarded against access from multiple threads, so it’s not clear to me how use of a StringView of one of those strings could be safe. I would like to look at that case at some point. 1234937 7 mark.lam 2016-09-29 13:18:26 -0700 (In reply to comment #4) > Comment on attachment 290119 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=290119&action=review > > > Source/WTF/wtf/text/StringView.cpp:240 > > + std::lock_guard<StaticLock> lock(underlyingStringsMutex); > > if (!--m_underlyingString->refCount) { > > Can we make UnderlyingString::refCount an atomic instead? I am concerned > that the locking will slow things down too much. UnderlyingString::refCount is already an atomic. The issue is that the adjustment of refCount and the insertion/removal from the underlyingStrings map needs to be an atomic unit. If the 2 are not in an atomic unit, then another thread can inc the refCount while we're waiting for the underlyingStringsMutex for the deletion. I supposed I can check the refCount again after acquiring the lock to decide whether to commence with removal from the map and deletion, or to skip them. I'll look into that. (In reply to comment #6) > I also suspect that in the original case where this was failing, there is > actually a thread safety problem in the client code, and the root cause is > not the lack of thread safety in StringView itself. After all, the reference > counts of the strings themselves are not guarded against access from > multiple threads, so it’s not clear to me how use of a StringView of one of > those strings could be safe. > > I would like to look at that case at some point. I'm not sure that this is true. The refCount of the UnderlyingString record is not tied to the refCount of the StringImpl that this StringView is associated with. In the test cases that were failing, the relevant StringImpl is that of a StringSourceProvider (i.e. very long lived relative to the StringViews). It's just that StringViews were being constructed and destructed many times from different threads, and with that comes the creation and deletion of the associated UnderlyingString record. Based on the evidence I've seen so far, it seems clear to me that the issue really is in StringView::adoptUnderlyingString() management of the UnderlyingString record, and does not have anything to do with the associated StringImpl. 1234968 8 mark.lam 2016-09-29 14:13:43 -0700 (In reply to comment #7) > If the 2 are not in an atomic unit, then another thread can inc the refCount > while we're waiting for the underlyingStringsMutex for the deletion. I > supposed I can check the refCount again after acquiring the lock to decide > whether to commence with removal from the map and deletion, or to skip them. > I'll look into that. This won't work. Imagine this scenario: 1. Thread T1 acquires and holds on to the underlyingStringsMutex. 2. Thread T2 has a StringView associated with StringImpl S2 and UnderlyingString U2, and is done with the StringView. T2 decrements UnderlyingString U2's refCount to 0 and wants to delete it, but blocks (in adoptUnderlyingString()) waiting for the underlyingStringsMutex owned by T1. 3. Thread T3 wants a StringView associated with StringImpl S2, but blocks (in setUnderlyingString()) waiting for the underlyingStringsMutex owned by T1. 4. T1 releases the underlyingStringsMutex. 5. T3 acquires the underlyingStringsMutex. 6. T3 queries the map and finds U2 associated with S2 (because T2 is still blocked and hasn't removed U2 from the map yet). Note: U2's refCount is already 0 because decremented it to 0 back in step 2 above. 7. T3 increments U2's refCount. T3 releases the underlyingStringsMutex. T3 is done with its StringView and destructs it. T3 acquires the underlyingStringsMutex. T3 decrements U2's refCount to 0, and deletes U2. T3 releases the underlyingStringsMutex. 8. T2 finally unblocks and acquires the underlyingStringsMutex. T2 checks U2's refCount and sees that it is 0 (because T3 decrement it back to 0 too) <==== Use after free right here. T2 tries to remove U2 from the map but fails because U2 is not in the map). T2 deleted U2. <======= Double delete. Another variation on this is Thread T4 shows up between step 7 and 8, and allocates a new UnderlyingString U4. But since T3 already deleted U2, the memory for U2 gets re-allocated as U4 now, and gets re-inserted to the map. Later in step 8, T2 actually delete U4 (because it has the same memory address as the deleted U2). Eventually, T4 tries to delete U4 and crashes. The only way to avoid this is to ensure that if ever decrement an UnderlyingString's refCount to 0, that UnderlyingString must also be removed from the map (atomically with the refCount decrement) to prevent it from being used by another thread. Oh wait, we could change StringView::setUnderlyingString() to check if the found UnderlyingString in the may has a refCount of 0. If so, this means it is in the process of being removed and deleted. As such, we can remove if from the map there and make a new one as if we didn't find it in the map. Let me try that. 1234976 9 mark.lam 2016-09-29 14:25:18 -0700 (In reply to comment #8) > Oh wait, we could change StringView::setUnderlyingString() to check if the > found UnderlyingString in the may has a refCount of 0. If so, this means it > is in the process of being removed and deleted. As such, we can remove if > from the map there and make a new one as if we didn't find it in the map. > Let me try that. No, that won't work either because the pending deletion and removal will do the removal from using the StringImpl* as the key. By then, the entry in the map can be for a different UnderlyingString. We could continue this dance and first check if the would be removed UnderlyingString pointer value from the map is the same as the one we expect. If it's the same, then we can proceed with the removal. What all this means is: if we want to avoid that lock on deletion, we'll have to: 1. make StringView::adoptUnderlyingString() second guess whether it can really delete the UnderlyingString record, and 2. make StringView::setUnderlyingString() second guess whether it has really found an existing UnderlyingString record in the map. This code is a lot more tricky. All it does is make the refCount decrements faster for refCounts greater than 1. It also makes all removal of the UnderlyingString record from the map slower because we're now making it do 2 map lookups: one to check if the record has been replaced, one to remove it (if not replaced). The setUnderlyingString() side will also be slower when UnderlyingString record already exists in the map. Hence, the code gets a lot more tricky, and I'm not sure that it is a net win. 1235042 10 darin 2016-09-29 17:01:57 -0700 OK, I guess we shouldn’t spend more time on this if it’s fast enough to be bearable. 290119 2016-09-28 14:11:18 -0700 2016-09-28 14:29:17 -0700 proposed patch. bug-162702.patch text/plain 6098 mark.lam SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAyMDY1NDIpCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDgzIEBACisyMDE2LTA5LTI4ICBNYXJrIExhbSAgPG1hcmsubGFt QGFwcGxlLmNvbT4KKworICAgICAgICBGaXggcmFjZSBjb25kaXRpb24gaW4gU3RyaW5nVmlldydz IFVuZGVybHlpbmdTdHJpbmcgbGlmZWN5Y2xlIG1hbmFnZW1lbnQuCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjI3MDIKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGVyZSAyIHJlbGV2YW50IGZ1bmN0aW9u cyBhdCBwbGF5OgorCisgICAgICAgIHZvaWQgU3RyaW5nVmlldzo6c2V0VW5kZXJseWluZ1N0cmlu Zyhjb25zdCBTdHJpbmdJbXBsKiBzdHJpbmcpCisgICAgICAgIHsKKyAgICAgICAgICAgIFVuZGVy bHlpbmdTdHJpbmcqIHVuZGVybHlpbmdTdHJpbmc7CisgICAgICAgICAgICBpZiAoIXN0cmluZykK KyAgICAgICAgICAgICAgICB1bmRlcmx5aW5nU3RyaW5nID0gbnVsbHB0cjsKKyAgICAgICAgICAg IGVsc2UgeworICAgICAgICAgICAgICAgIHN0ZDo6bG9ja19ndWFyZDxTdGF0aWNMb2NrPiBsb2Nr KHVuZGVybHlpbmdTdHJpbmdzTXV0ZXgpOworICAgICAgICAgICAgICAgIGF1dG8gcmVzdWx0ID0g dW5kZXJseWluZ1N0cmluZ3MoKS5hZGQoc3RyaW5nLCBudWxscHRyKTsKKyAgICAgICAgICAgICAg ICBpZiAocmVzdWx0LmlzTmV3RW50cnkpCisgICAgICAgICAgICAgICAgICAgIHJlc3VsdC5pdGVy YXRvci0+dmFsdWUgPSBuZXcgVW5kZXJseWluZ1N0cmluZygqc3RyaW5nKTsKKyAgICAgICAgICAg ICAgICBlbHNlCisgICAgICAgICAgICAgICAgICAgICsrcmVzdWx0Lml0ZXJhdG9yLT52YWx1ZS0+ cmVmQ291bnQ7CisgICAgICAgICAgICAgICAgdW5kZXJseWluZ1N0cmluZyA9IHJlc3VsdC5pdGVy YXRvci0+dmFsdWU7IC8vIFBvaW50IFAyLgorICAgICAgICAgICAgfQorICAgICAgICAgICAgYWRv cHRVbmRlcmx5aW5nU3RyaW5nKHVuZGVybHlpbmdTdHJpbmcpOyAvLyBQb2ludCBQNS4KKyAgICAg ICAgfQorCisgICAgICAgIC4uLiBhbmQgLi4uCisKKyAgICAgICAgdm9pZCBTdHJpbmdWaWV3Ojph ZG9wdFVuZGVybHlpbmdTdHJpbmcoVW5kZXJseWluZ1N0cmluZyogdW5kZXJseWluZ1N0cmluZykK KyAgICAgICAgeworICAgICAgICAgICAgaWYgKG1fdW5kZXJseWluZ1N0cmluZykgeworICAgICAg ICAgICAgICAgIC8vIFBvaW50IFAwLgorICAgICAgICAgICAgICAgIGlmICghLS1tX3VuZGVybHlp bmdTdHJpbmctPnJlZkNvdW50KSB7CisgICAgICAgICAgICAgICAgICAgIGlmIChtX3VuZGVybHlp bmdTdHJpbmctPmlzVmFsaWQpIHsgLy8gUG9pbnQgUDEuCisgICAgICAgICAgICAgICAgICAgICAg ICBzdGQ6OmxvY2tfZ3VhcmQ8U3RhdGljTG9jaz4gbG9jayh1bmRlcmx5aW5nU3RyaW5nc011dGV4 KTsKKyAgICAgICAgICAgICAgICAgICAgICAgIHVuZGVybHlpbmdTdHJpbmdzKCkucmVtb3ZlKCZt X3VuZGVybHlpbmdTdHJpbmctPnN0cmluZyk7IC8vIFBvaW50IFAzLgorICAgICAgICAgICAgICAg ICAgICB9CisgICAgICAgICAgICAgICAgICAgIGRlbGV0ZSBtX3VuZGVybHlpbmdTdHJpbmc7IC8v IFBvaW50IFA0LgorICAgICAgICAgICAgICAgIH0KKyAgICAgICAgICAgIH0KKyAgICAgICAgICAg IG1fdW5kZXJseWluZ1N0cmluZyA9IHVuZGVybHlpbmdTdHJpbmc7CisgICAgICAgIH0KKworICAg ICAgICBJbWFnaW5lIHRoZSBmb2xsb3dpbmcgc2NlbmFyaW86CisKKyAgICAgICAgMS4gVGhyZWFk IFQxIGhhcyBiZWVuIHVzaW5nIGFuIFVuZGVybHlpbmdTdHJpbmcgVTEsIGFuZCBpcyBub3cgZG9u ZSB3aXRoIGl0LgorICAgICAgICAgICBUMSBydW5zIHVwIHRvIHBvaW50IFAxIGluIGFkb3B0VW5k ZXJseWluZ1N0cmluZygpLCBhbmQgaXMgYmxvY2tlZCB3YWl0aW5nIGZvcgorICAgICAgICAgICB0 aGUgdW5kZXJseWluZ1N0cmluZ3NNdXRleCAod2hpY2ggaXMgY3VycmVudGx5IGJlaW5nIGhlbGQg YnkgVGhyZWFkIFQyKS4KKyAgICAgICAgMi4gQ29udGV4dCBzd2l0Y2ggdG8gVGhyZWFkIFQyLgor ICAgICAgICAgICBUMiB3YW50cyB0byB1c2UgVW5kZXJseWluZ1N0cmluZyBVMSwgYW5kIHJ1bnMg dXAgdG8gcG9pbnQgUDIgaW4gc2V0VW5kZXJseWluZ1N0cmluZygpCisgICAgICAgICAgIGFuZCBy ZWxlYXNlcyB0aGUgdW5kZXJseWluZ1N0cmluZ3NNdXRleC4KKyAgICAgICAgICAgTm90ZTogVDIg dGhpbmtzIGl0IGhhcyBzdWNjZXNzZnVsbHkgcmVmQ291bnRlZCBVMSwgYW5kIHRoZXJlZm9yZSBV MSBpcyBzYWZlIHRvIHVzZS4KKyAgICAgICAgMy4gQ29udGV4dCBzd2l0Y2ggdG8gVGhyZWFkIFQx LgorICAgICAgICAgICBUMSBhY3F1aXJlcyB0aGUgdW5kZXJseWluZ1N0cmluZ3NNdXRleCwgYW5k IHByb2NlZWRzIHRvIHJlbW92ZSBpdCBmcm9tIHRoZQorICAgICAgICAgICB1bmRlcmx5aW5nU3Ry aW5ncygpIG1hcCAoc2VlIFBvaW50IFAzKS4gIEl0IHRoaW5rcyBpdCBoYXMgc3VjY2Vzc2Z1bGx5 IGRvbmUgc28KKyAgICAgICAgICAgYW5kIHByb2NlZWRzIHRvIGRlbGV0ZSBVMSAoc2VlIFBvaW50 IFA0KS4KKyAgICAgICAgNC4gQ29udGV4dCBzd2l0Y2ggdG8gVGhyZWFkIFQyLgorICAgICAgICAg ICBUMiBwcm9jZWVkcyB0byB1c2UgVTEgKHNlZSBQb2ludCBQNSBpbiBzZXRVbmRlcmx5aW5nU3Ry aW5nKCkpLgorICAgICAgICAgICBOb3RlOiBVMSBoYXMgYWxyZWFkeSBiZWVuIGZyZWVkLiAgVGhp cyBpcyBhIHVzZSBhZnRlciBmcmVlLgorCisgICAgICAgIFRoZSBmaXggaXMgdG8gYWNxdWlyZSB0 aGUgdW5kZXJseWluZ1N0cmluZ3NNdXRleCBhdCBQb2ludCBQMCBpbiBhZG9wdFVuZGVybHlpbmdT dHJpbmcoKQorICAgICAgICBpbnN0ZWFkIG9mIGFmdGVyIFAxLiAgVGhpcyBlbnN1cmVzIHRoYXQg dGhlIGRlY3JlbWVudGluZyBvZiB0aGUgVW5kZXJseWluZ1N0cmluZworICAgICAgICByZWZDb3Vu dCBhbmQgaXRzIHJlbW92YWwgZnJvbSB0aGUgdW5kZXJseWluZ1N0cmluZ3MoKSBtYXAgaXMgZG9u ZSBhcyBhbiBhdG9taWMgdW5pdC4KKworICAgICAgICBOb3RlOiBJZiB5b3UgbG9vayBpbiBTdHJp bmdWaWV3LmNwcCwgeW91IHNlZSBhbm90aGVyIHNldFVuZGVybHlpbmdTdHJpbmcoKSB3aGljaAor ICAgICAgICB0YWtlcyBhIFN0cmluZ1ZpZXcgb3RoZXJTdHJpbmcuICBUaGlzIHZlcnNpb24gb2Yg c2V0VW5kZXJseWluZ1N0cmluZygpIGNhbiBvbmx5CisgICAgICAgIGJlIGNhbGxlZCBmcm9tIHdp dGhpbiB0aGUgc2FtZSB0aHJlYWQgdGhhdCBjcmVhdGVkIHRoZSBvdGhlciBTdHJpbmdWaWV3LiAg QXMgYQorICAgICAgICByZXN1bHQsIGhlcmUsIHdlIGFyZSBndWFyYW50ZWVkIHRoYXQgdGhlIFVu ZGVybHlpbmdTdHJpbmcgcmVmQ291bnQgaXMgbmV2ZXIgemVybywKKyAgICAgICAgYW5kIHRoZXJl J3Mgbm8gb3RoZXIgdGhyZWF0IG9mIGFub3RoZXIgdGhyZWFkIHRyeWluZyB0byBkZWxldGUgdGhl IFVuZGVybHlpbmdTdHJpbmcKKyAgICAgICAgd2hpbGUgd2UgYWRvcHQgaXQuICBIZW5jZSwgd2Ug ZG9uJ3QgbmVlZCB0byBhY3F1aXJlIHRoZSB1bmRlcmx5aW5nU3RyaW5nc011dGV4CisgICAgICAg IGhlcmUuCisKKyAgICAgICAgVGhpcyByYWNlIGNvbmRpdGlvbiB3YXMgZm91bmQgd2hlbiBydW5u aW5nIGxheW91dCB0ZXN0cyBmZXRjaC9mZXRjaC13b3JrZXItY3Jhc2guaHRtbAorICAgICAgICBh bmQgc3RvcmFnZS9pbmRleGVkZGIvbW9kZXJuL29wZW5kYXRhYmFzZS12ZXJzaW9ucy5odG1sIHdo ZW4gQ0hFQ0tfU1RSSU5HVklFV19MSUZFVElNRQorICAgICAgICBpcyBlbmFibGVkLiAgVGhpcyBp c3N1ZSByZXN1bHRlZCBpbiB0aG9zZSB0ZXN0cyBjcmFzaGluZyBkdWUgdG8gYSB1c2UtYWZ0ZXIt ZnJlZS4KKworICAgICAgICAqIHd0Zi90ZXh0L1N0cmluZ1ZpZXcuY3BwOgorICAgICAgICAoV1RG OjpTdHJpbmdWaWV3OjphZG9wdFVuZGVybHlpbmdTdHJpbmcpOgorICAgICAgICAoV1RGOjpTdHJp bmdWaWV3OjpzZXRVbmRlcmx5aW5nU3RyaW5nKToKKwogMjAxNi0wOS0yOCAgQnJlbnQgRnVsZ2hh bSAgPGJmdWxnaGFtQGFwcGxlLmNvbT4KIAogICAgICAgICBDb3JyZWN0ICdzYWZlQ2FzdCcgaW1w bGVtZW50YXRpb24KSW5kZXg6IFNvdXJjZS9XVEYvd3RmL3RleHQvU3RyaW5nVmlldy5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL1dURi93dGYvdGV4dC9TdHJpbmdWaWV3LmNwcAkocmV2aXNpb24g MjA2NTQxKQorKysgU291cmNlL1dURi93dGYvdGV4dC9TdHJpbmdWaWV3LmNwcAkod29ya2luZyBj b3B5KQpAQCAtMSw2ICsxLDYgQEAKIC8qCiAKLUNvcHlyaWdodCAoQykgMjAxNCBBcHBsZSBJbmMu IEFsbCByaWdodHMgcmVzZXJ2ZWQuCitDb3B5cmlnaHQgKEMpIDIwMTQsIDIwMTYgQXBwbGUgSW5j LiBBbGwgcmlnaHRzIHJlc2VydmVkLgogCiBSZWRpc3RyaWJ1dGlvbiBhbmQgdXNlIGluIHNvdXJj ZSBhbmQgYmluYXJ5IGZvcm1zLCB3aXRoIG9yIHdpdGhvdXQKIG1vZGlmaWNhdGlvbiwgYXJlIHBl cm1pdHRlZCBwcm92aWRlZCB0aGF0IHRoZSBmb2xsb3dpbmcgY29uZGl0aW9ucwpAQCAtMjM2LDkg KzIzNiw5IEBAIGJvb2wgU3RyaW5nVmlldzo6dW5kZXJseWluZ1N0cmluZ0lzVmFsaWQKIHZvaWQg U3RyaW5nVmlldzo6YWRvcHRVbmRlcmx5aW5nU3RyaW5nKFVuZGVybHlpbmdTdHJpbmcqIHVuZGVy bHlpbmdTdHJpbmcpCiB7CiAgICAgaWYgKG1fdW5kZXJseWluZ1N0cmluZykgeworICAgICAgICBz dGQ6OmxvY2tfZ3VhcmQ8U3RhdGljTG9jaz4gbG9jayh1bmRlcmx5aW5nU3RyaW5nc011dGV4KTsK ICAgICAgICAgaWYgKCEtLW1fdW5kZXJseWluZ1N0cmluZy0+cmVmQ291bnQpIHsKICAgICAgICAg ICAgIGlmIChtX3VuZGVybHlpbmdTdHJpbmctPmlzVmFsaWQpIHsKLSAgICAgICAgICAgICAgICBz dGQ6OmxvY2tfZ3VhcmQ8U3RhdGljTG9jaz4gbG9jayh1bmRlcmx5aW5nU3RyaW5nc011dGV4KTsK ICAgICAgICAgICAgICAgICB1bmRlcmx5aW5nU3RyaW5ncygpLnJlbW92ZSgmbV91bmRlcmx5aW5n U3RyaW5nLT5zdHJpbmcpOwogICAgICAgICAgICAgfQogICAgICAgICAgICAgZGVsZXRlIG1fdW5k ZXJseWluZ1N0cmluZzsKQEAgLTI2NCw5ICsyNjQsMTMgQEAgdm9pZCBTdHJpbmdWaWV3OjpzZXRV bmRlcmx5aW5nU3RyaW5nKGNvbgogICAgIGFkb3B0VW5kZXJseWluZ1N0cmluZyh1bmRlcmx5aW5n U3RyaW5nKTsKIH0KIAotdm9pZCBTdHJpbmdWaWV3OjpzZXRVbmRlcmx5aW5nU3RyaW5nKGNvbnN0 IFN0cmluZ1ZpZXcmIHN0cmluZykKK3ZvaWQgU3RyaW5nVmlldzo6c2V0VW5kZXJseWluZ1N0cmlu Zyhjb25zdCBTdHJpbmdWaWV3JiBvdGhlclN0cmluZykKIHsKLSAgICBVbmRlcmx5aW5nU3RyaW5n KiB1bmRlcmx5aW5nU3RyaW5nID0gc3RyaW5nLm1fdW5kZXJseWluZ1N0cmluZzsKKyAgICAvLyBT dHJpbmdWaWV3IGluc3RhbmNlcyBhcmUgbm90IHBhc3NlZCBhY3Jvc3MgdGhyZWFkcy4gSGVuY2Us IHNpbmNlIHdlIGhhdmUgYQorICAgIC8vIHJlZmVyZW5jZSB0byBvdGhlclN0cmluZywgd2UgYXJl IGd1YXJhbnRlZWQgdGhhdCBpdHMgdW5kZXJseWluZ1N0cmluZyByZWZDb3VudAorICAgIC8vIHdp bGwgcmVtYWluIG5vbi16ZXJvIGZvciB1cyB0byBpbmNyZW1lbnQuIFdlIGRvbid0IG5lZWQgdG8g Z3JhYiB0aGUKKyAgICAvLyB1bmRlcmx5aW5nU3RyaW5nc011dGV4LgorICAgIFVuZGVybHlpbmdT dHJpbmcqIHVuZGVybHlpbmdTdHJpbmcgPSBvdGhlclN0cmluZy5tX3VuZGVybHlpbmdTdHJpbmc7 CiAgICAgaWYgKHVuZGVybHlpbmdTdHJpbmcpCiAgICAgICAgICsrdW5kZXJseWluZ1N0cmluZy0+ cmVmQ291bnQ7CiAgICAgYWRvcHRVbmRlcmx5aW5nU3RyaW5nKHVuZGVybHlpbmdTdHJpbmcpOwo=