Bug 118613 - [CoordinatedGraphics] Crash at CoordinatedGraphicsScene::updateImageBacking
Summary: [CoordinatedGraphics] Crash at CoordinatedGraphicsScene::updateImageBacking
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Luciano Wolf
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-12 14:20 PDT by Luciano Wolf
Modified: 2022-07-29 10:53 PDT (History)
7 users (show)

See Also:


Attachments
Proposed fix (1.05 KB, patch)
2013-07-12 14:26 PDT, Luciano Wolf
no flags Details | Formatted Diff | Diff
Proposed fix. (2.06 KB, patch)
2013-07-15 09:31 PDT, Luciano Wolf
noam: review+
noam: commit-queue-
Details | Formatted Diff | Diff
Proposed fix. (2.17 KB, patch)
2013-07-15 10:07 PDT, Luciano Wolf
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luciano Wolf 2013-07-12 14:20:49 PDT
If you run this demo: http://dev.sencha.com/deploy/touch/examples/production/kitchensink/#demo/touchevents and click on "Console" button, it produces a segmentation fault:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6491d85 in WebCore::CoordinatedGraphicsScene::updateImageBacking(unsigned long, WTF::PassRefPtr<WebCore::CoordinatedSurface>) ()

It was tested with Nix (MiniBrowser --ipad) and with Qt (MiniBrowser --user-agent "Mozilla/5.0 (iPad; U; CPU OS 3_2 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Version/4.0.4 Mobile/7B334b Safari/531.21.10")


I was able to fix the problem but I'm not sure if it's the right fix. Inside CoordinatedGraphicsScene::syncImageBackings(const CoordinatedGraphicsState& state) function there are some iterations to deal with images that must be created, removed, updated and cleared. The thing is that it tries to create/remove/update the very same ID thus generating a segfault - as it tries to updated an ID that was killed. I can "fix" the problem just changing the order of these operations: execute the removals prior any other operation.

I'm not considering this as a proper fix because it's weird to have the same ID scheduled for all operations. What do you guys think?
Comment 1 Luciano Wolf 2013-07-12 14:26:39 PDT
Created attachment 206572 [details]
Proposed fix
Comment 2 Luciano Wolf 2013-07-15 09:31:35 PDT
Created attachment 206669 [details]
Proposed fix.
Comment 3 Noam Rosenthal 2013-07-15 09:54:49 PDT
Comment on attachment 206669 [details]
Proposed fix.

View in context: https://bugs.webkit.org/attachment.cgi?id=206669&action=review

> Source/WebCore/ChangeLog:9
> +        operation - solves the issue.
> +

You should mention why :)
It works because when an image is replaced the old one is deleted before the new one is created, and IDs are not unique in cairo (they're based on the image pointer)
Comment 4 Luciano Wolf 2013-07-15 10:07:39 PDT
Created attachment 206673 [details]
Proposed fix.
Comment 5 WebKit Commit Bot 2013-07-15 10:50:24 PDT
Comment on attachment 206673 [details]
Proposed fix.

Clearing flags on attachment: 206673

Committed r152647: <http://trac.webkit.org/changeset/152647>
Comment 6 Noam Rosenthal 2013-08-02 01:42:26 PDT
This crash still appears, the patch makes it more rare but doesn't fix it.
The right fix for this is to stop using the Cairo pointer as an image ID, but rather have a map in the WebProcess between a referenced cairo surface and an image ID.
Comment 7 Noam Rosenthal 2013-08-02 01:42:56 PDT
See previous comment.
Comment 8 Alexey Proskuryakov 2022-07-29 10:53:31 PDT
The Qt port has been removed from WebKit, resolving this bug.