233014 2021-11-11 14:12:38 -0800 Fix for crash in LayoutTests in isolated tree mode. 2021-11-15 16:50:52 -0800 1 1 1 Unclassified WebKit Accessibility WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 andresg_22 andresg_22 andresg_22 cfleizach darin webkit-bug-importer oldest_to_newest 1814219 0 andresg_22 2021-11-11 14:12:38 -0800 Fix for crash in LayoutTests in isolated tree mode. 1814220 1 webkit-bug-importer 2021-11-11 14:12:52 -0800 <rdar://problem/85315168> 1814222 2 444000 andresg_22 2021-11-11 14:18:05 -0800 Created attachment 444000 Patch 1814272 3 ews-feeder 2021-11-11 15:38:41 -0800 Committed r285677 (244160@main): <https://commits.webkit.org/244160@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 444000. 1814287 4 444000 darin 2021-11-11 16:18:35 -0800 Comment on attachment 444000 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444000&action=review > Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:582 > - NSString *value = descriptionOfValue(attributeValue(attribute).get()); > - [values appendFormat:@"%@: %@\n", attribute, value]; > + RetainPtr<NSString> value = descriptionOfValue(attributeValue(attribute).get()); > + [values appendFormat:@"%@: %@\n", attribute, value.get()]; I don’t understand this. Why do we need to retain the string? 1815282 5 andresg_22 2021-11-15 16:50:52 -0800 (In reply to Darin Adler from comment #4) > Comment on attachment 444000 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444000&action=review > > > Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:582 > > - NSString *value = descriptionOfValue(attributeValue(attribute).get()); > > - [values appendFormat:@"%@: %@\n", attribute, value]; > > + RetainPtr<NSString> value = descriptionOfValue(attributeValue(attribute).get()); > > + [values appendFormat:@"%@: %@\n", attribute, value.get()]; > > I don’t understand this. Why do we need to retain the string? Darin, I don't have a good explanation for this, but this doesn't crash any longer. The other way I found to avoid this crash is as follows: - NSString *description = descriptionOfValue(attributeValue(NSAccessibilityValueAttribute).get()); // the above crashes when you dereference description in any way, like description.length. + auto value = attributeValue(NSAccessibilityValueAttribute); + NSString *description = descriptionOfValue(value.get()); // doesn't crash if you description.length. attributeValue is doing some threading trickery because it is dispatching to a secondary, mocked thread and waiting, and then spinning the main thread run loop until the dispatched block is executed on the secondary thread. My hypothesis is that this is allowing the garbage collector to kick in and release the return value of attributeValue, unless is assigned to an lvalue. The stack trace of the crash is not inside descriptionOfValue though, but when dereferencing the description string which makes no much sense to me... would greatly appreciate it if you had any suggestion. Thanks! 444000 2021-11-11 14:18:05 -0800 2021-11-11 15:38:41 -0800 Patch bug-233014-20211111171804.patch text/plain 1964 andresg_22 U3VidmVyc2lvbiBSZXZpc2lvbjogMjg1NTIxCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMmU0ZTkwNTg1ODRkNTEyNDljMTM0NWM2YjY4ZTVmMTZj Mjg2OWQ1ZC4uY2MyMWU5MzlkNmYwYjE5NGMwZjliZTZjZjdlMWQyYjUyZDliZDkzZCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIw IEBACisyMDIxLTExLTExICBBbmRyZXMgR29uemFsZXogIDxhbmRyZXNnXzIyQGFwcGxlLmNvbT4K KworICAgICAgICBGaXggZm9yIGNyYXNoIGluIExheW91dFRlc3RzIGluIGlzb2xhdGVkIHRyZWUg bW9kZS4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIz MzAxNAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRo aXMgZml4ZXMgdGhlIGZvbGxvd2luZyB0ZXN0cyBpbiBpc29sYXRlZCB0cmVlIG1vZGU6CisgICAg ICAgIGFjY2Vzc2liaWxpdHkvaW1hZ2UtbGluay1pbmxpbmUtY29udC5odG1sIFsgQ3Jhc2ggXQor ICAgICAgICBhY2Nlc3NpYmlsaXR5L2ltYWdlLWxpbmsuaHRtbCBbIENyYXNoIF0KKyAgICAgICAg YWNjZXNzaWJpbGl0eS9pbnRlcm5hbC1saW5rLWFuY2hvcnMyLmh0bWwgWyBDcmFzaCBdCisKKyAg ICAgICAgKiBXZWJLaXRUZXN0UnVubmVyL0luamVjdGVkQnVuZGxlL21hYy9BY2Nlc3NpYmlsaXR5 VUlFbGVtZW50TWFjLm1tOgorICAgICAgICAoV1RSOjpBY2Nlc3NpYmlsaXR5VUlFbGVtZW50Ojph bGxBdHRyaWJ1dGVzKToKKyAgICAgICAgTmVlZCB0byByZXRhaW4gdGhlIHBvaW50ZXIgcmV0dXJu ZWQgYnkKKyAgICAgICAgQWNjZXNzaWJpbGl0eVVJRWxlbWVudDo6ZGVzY3JpcHRpb25PZlZhbHVl LgorCiAyMDIxLTExLTA5ICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAgICAg ICAgTmV3IHNwZWM6IEJsb2NrIGV4dGVybmFsIHByb3RvY29sIGhhbmRsZXIgaW4gc2FuZGJveGVk IGZyYW1lcwpkaWZmIC0tZ2l0IGEvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9JbmplY3RlZEJ1bmRs ZS9tYWMvQWNjZXNzaWJpbGl0eVVJRWxlbWVudE1hYy5tbSBiL1Rvb2xzL1dlYktpdFRlc3RSdW5u ZXIvSW5qZWN0ZWRCdW5kbGUvbWFjL0FjY2Vzc2liaWxpdHlVSUVsZW1lbnRNYWMubW0KaW5kZXgg OGZmOWQ3ZjBiYWJkZWVhYjFiNDI1NTE3ZDQ4ZjllNDE1YTk1NzcxMS4uNjRkNTFjMWE0ZjhhMjY4 ZTMxZmY4ZDQ1MTU4OWViYTAxODVmYWU3ZSAxMDA2NDQKLS0tIGEvVG9vbHMvV2ViS2l0VGVzdFJ1 bm5lci9JbmplY3RlZEJ1bmRsZS9tYWMvQWNjZXNzaWJpbGl0eVVJRWxlbWVudE1hYy5tbQorKysg Yi9Ub29scy9XZWJLaXRUZXN0UnVubmVyL0luamVjdGVkQnVuZGxlL21hYy9BY2Nlc3NpYmlsaXR5 VUlFbGVtZW50TWFjLm1tCkBAIC01NzgsOCArNTc4LDggQEAgSlNSZXRhaW5QdHI8SlNTdHJpbmdS ZWY+IEFjY2Vzc2liaWxpdHlVSUVsZW1lbnQ6OmFsbEF0dHJpYnV0ZXMoKQogICAgICAgICAgICAg fHwgW2F0dHJpYnV0ZSBpc0VxdWFsVG9TdHJpbmc6QCJBWFJlbGF0aXZlRnJhbWUiXSkKICAgICAg ICAgICAgIGNvbnRpbnVlOwogCi0gICAgICAgIE5TU3RyaW5nICp2YWx1ZSA9IGRlc2NyaXB0aW9u T2ZWYWx1ZShhdHRyaWJ1dGVWYWx1ZShhdHRyaWJ1dGUpLmdldCgpKTsKLSAgICAgICAgW3ZhbHVl cyBhcHBlbmRGb3JtYXQ6QCIlQDogJUBcbiIsIGF0dHJpYnV0ZSwgdmFsdWVdOworICAgICAgICBS ZXRhaW5QdHI8TlNTdHJpbmc+IHZhbHVlID0gZGVzY3JpcHRpb25PZlZhbHVlKGF0dHJpYnV0ZVZh bHVlKGF0dHJpYnV0ZSkuZ2V0KCkpOworICAgICAgICBbdmFsdWVzIGFwcGVuZEZvcm1hdDpAIiVA OiAlQFxuIiwgYXR0cmlidXRlLCB2YWx1ZS5nZXQoKV07CiAgICAgfQogCiAgICAgcmV0dXJuIFt2 YWx1ZXMgY3JlYXRlSlNTdHJpbmdSZWZdOwo=