Summary: | Crash in [RBSTarget targetWithPid:] during WebProcessProxy::shutDown | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ali Juma <ajuma> | ||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, cdumez, darin, ggaren, kkinnunen, sihui_liu, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ali Juma
2021-08-24 08:29:39 PDT
I have just discussed this with a RunningBoard engineer. The most likely explanation is that the pid is < 0 (which is possible since pid_t is an int on our Darwin). We get the pid from xpc_connection_get_pid() and I guess this could theoretically return a negative PID (maybe in case of error when the process has already exited). I think we should tweak the PID check on WebKit side to early return if pid <= 0 (instead of doing an early return when !pid). Are you able to write up the patch? (In reply to Chris Dumez from comment #1) > I have just discussed this with a RunningBoard engineer. The most likely > explanation is that the pid is < 0 (which is possible since pid_t is an int > on our Darwin). We get the pid from xpc_connection_get_pid() and I guess > this could theoretically return a negative PID (maybe in case of error when > the process has already exited). > > I think we should tweak the PID check on WebKit side to early return if pid > <= 0 (instead of doing an early return when !pid). > > Are you able to write up the patch? By the way, I haven't received such reports for MobileSafari yet, which is a bit odd. I have no idea what Chrome could be doing differently to cause this... (In reply to Chris Dumez from comment #2) > (In reply to Chris Dumez from comment #1) > > I have just discussed this with a RunningBoard engineer. The most likely > > explanation is that the pid is < 0 (which is possible since pid_t is an int > > on our Darwin). We get the pid from xpc_connection_get_pid() and I guess > > this could theoretically return a negative PID (maybe in case of error when > > the process has already exited). > > > > I think we should tweak the PID check on WebKit side to early return if pid > > <= 0 (instead of doing an early return when !pid). > > > > Are you able to write up the patch? > > By the way, I haven't received such reports for MobileSafari yet, which is a > bit odd. I have no idea what Chrome could be doing differently to cause > this... Do you know when this crash started for Chrome? And could you give us an idea of the frequency (i.e. is it a top crash?). Created attachment 436298 [details]
Patch
(In reply to Chris Dumez from comment #3) > (In reply to Chris Dumez from comment #2) > > (In reply to Chris Dumez from comment #1) > > > I have just discussed this with a RunningBoard engineer. The most likely > > > explanation is that the pid is < 0 (which is possible since pid_t is an int > > > on our Darwin). We get the pid from xpc_connection_get_pid() and I guess > > > this could theoretically return a negative PID (maybe in case of error when > > > the process has already exited). > > > > > > I think we should tweak the PID check on WebKit side to early return if pid > > > <= 0 (instead of doing an early return when !pid). > > > > > > Are you able to write up the patch? > > > > By the way, I haven't received such reports for MobileSafari yet, which is a > > bit odd. I have no idea what Chrome could be doing differently to cause > > this... > > Do you know when this crash started for Chrome? And could you give us an > idea of the frequency (i.e. is it a top crash?). We started getting this crash in iOS 14.0 (ideally we'd have filed a WebKit bug earlier but it somehow fell through the cracks until now). It represents about 0.22% of all crashes for the current version of Chrome, so it's not a top crash but it's not completely rare either. Committed r281511 (240884@main): <https://commits.webkit.org/240884@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 436298 [details]. |