Bug 139868

Summary: Too large plugins are crashing (tryFastMalloc is broken with bmalloc)
Product: WebKit Reporter: Gabor Rapcsanyi <rgabor>
Component: Web Template FrameworkAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: ahmad.saleem792, andersca, ap, aroben, commit-queue, darin, ggaren, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 142443    
Bug Blocks:    
Attachments:
Description Flags
proposed fix
andersca: review-
proposed fix 2 none

Gabor Rapcsanyi
Reported 2014-12-22 11:06:48 PST
Too large plugins are crashing. Test case: <html> <object width="9999999999" data="test.swf"></object> </html>
Attachments
proposed fix (2.87 KB, patch)
2014-12-22 11:19 PST, Gabor Rapcsanyi 		andersca: review-
DetailsFormatted DiffDiff
proposed fix 2 (2.98 KB, patch)
2014-12-30 09:56 PST, Gabor Rapcsanyi 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Gabor Rapcsanyi
Comment 1 2014-12-22 11:19:52 PST
Created attachment 243629 [details] proposed fix
Anders Carlsson
Comment 2 2014-12-22 11:53:49 PST
Comment on attachment 243629 [details] proposed fix Patch looks good, but the test needs to use the test plug-in.
Gabor Rapcsanyi
Comment 3 2014-12-22 15:13:31 PST
(In reply to comment #2) > Comment on attachment 243629 [details] > proposed fix > > Patch looks good, but the test needs to use the test plug-in. Do you mean x-webkit-test-netscape? I tried it but this only happens with x-shockwave-flash. I can change it to <embed width="9999999999" src="foo.swf" type="application/x-shockwave-flash"> if you prefer that.
Anders Carlsson
Comment 4 2014-12-22 15:40:21 PST
(In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 243629 [details] > > proposed fix > > > > Patch looks good, but the test needs to use the test plug-in. > > Do you mean x-webkit-test-netscape? I tried it but this only happens with > x-shockwave-flash. > I can change it to > <embed width="9999999999" src="foo.swf" type="application/x-shockwave-flash"> > if you prefer that. My point is that we can't have tests that rely on Flash, because then we'd require people to install Flash. Maybe we can make the test plug-in crash somehow?
Gabor Rapcsanyi
Comment 5 2014-12-30 09:56:45 PST
Created attachment 243826 [details] proposed fix 2 This way its crashing with the test plug-in.
WebKit Commit Bot
Comment 6 2014-12-30 10:40:28 PST
Comment on attachment 243826 [details] proposed fix 2 Clearing flags on attachment: 243826 Committed r177824: <http://trac.webkit.org/changeset/177824>
WebKit Commit Bot
Comment 7 2014-12-30 10:40:31 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 8 2014-12-30 16:24:41 PST
The new test crashes on Mac with a RELEASE_ASSERT: https://build.webkit.org/results/Apple%20Yosemite%20Release%20WK2%20(Tests)/r177825%20(1624)/plugins/large-plugin-crash-crash-log.txt Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010867bc32 bmalloc::Heap::allocateXLarge(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long) + 98 1 com.apple.JavaScriptCore 0x000000010867a7e7 bmalloc::Allocator::allocateXLarge(unsigned long) + 71 2 com.apple.JavaScriptCore 0x000000010865a537 WTF::fastMalloc(unsigned long) + 151 3 com.apple.JavaScriptCore 0x000000010865a5b1 WTF::tryFastMalloc(unsigned long) + 17 EWS did see the problem, but the patch got landed before the bubble turned red. What's the next step here, should the patch be rolled out?
Alexey Proskuryakov
Comment 9 2014-12-31 00:40:54 PST
It's making bots way too red, and it's the only test for this fix, so skipping is not really appropriate. Rolling out.
Gabor Rapcsanyi
Comment 10 2015-02-16 04:04:05 PST
Now thats a problem on Gtk as well after enabling bmalloc. PassRefPtr<ShareableBitmap> ShareableBitmap::create(const IntSize& size, Flags flags) using tryFastMalloc to allocate memory but it seems if it can't allocate enough memory it will just crash instead of returning with null. I think we should implement the tryFastMalloc correctly or if we don't want to support this then rename it and clear the return value checks to prevent confusion.
Alexey Proskuryakov
Comment 11 2015-02-16 09:33:07 PST
So, we need to fixes to address this bug: 1. the fix to PluginProxy::updateBackingStore(), 2. and a fix to bmalloc to have tryFastMalloc actually work.
Radar WebKit Bug Importer
Comment 12 2015-02-16 09:33:23 PST
<rdar://problem/19846625>
Ahmad Saleem
Comment 13 2022-06-27 15:59:48 PDT
Attached patch was updated "PluginProxy.cpp" file, which is now removed from Webkit source (checked on Github), it was removed in following commit: https://github.com/WebKit/WebKit/commit/043ef2367e62a2cc7e9facb1bdc42b0867b8dd6d Is this applicable now? or this can be marked as "RESOLVED WONTFIX"? Thanks!
Note You need to log in before you can comment on or make changes to this bug.