Bug 98998

Summary: [WK2] Change the scope of locking in CoreIPC::Connection class.
Product: WebKit Reporter: Byungwoo Lee <bw80.lee>
Component: WebKit EFLAssignee: Byungwoo Lee <bw80.lee>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, laszlo.gombos, lucas.de.marchi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 98580    
Attachments:
Description Flags
Patch
none
Patch none

Byungwoo Lee
Reported 2012-10-10 23:56:48 PDT
locking range in the Connection::enqueueIncomingMessage() function need to be changed to protect lockup.
Attachments
Patch (2.77 KB, patch)
2012-10-11 20:02 PDT, Byungwoo Lee 		no flags
DetailsFormatted DiffDiff
Patch (2.52 KB, patch)
2012-10-30 21:43 PDT, Byungwoo Lee 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Byungwoo Lee
Comment 1 2012-10-11 20:02:40 PDT
Created attachment 168340 [details] Patch
Byungwoo Lee
Comment 2 2012-10-15 00:27:49 PDT
I tested layout test and unit test on EFL, and there was no regressions.
Laszlo Gombos
Comment 3 2012-10-22 22:37:43 PDT
This looks good to me. It make sense to release the lock immediately after the resource that is protected is no longer modified. Anders, can you take a look ?
Anders Carlsson
Comment 4 2012-10-23 12:58:26 PDT
I think the patch looks OK, but there seems to be a fundamental problem with EFL if you can get a deadlock in this case.
Laszlo Gombos
Comment 5 2012-10-23 13:25:16 PDT
(In reply to comment #4) > I think the patch looks OK, but there seems to be a fundamental problem with EFL if you can get a deadlock in this case. Thanks. There is indeed a problem with the EFL port. After the enablers in the common code (e.g. like this patch) are landed, the issue with the EFL port will be addressed.
Anders Carlsson
Comment 6 2012-10-23 18:39:11 PDT
> Thanks. > > There is indeed a problem with the EFL port. After the enablers in the common code (e.g. like this patch) are landed, the issue with the EFL port will be addressed. Wouldn't it be better to address the issue first?
Laszlo Gombos
Comment 7 2012-10-23 19:04:04 PDT
(In reply to comment #6) > > Thanks. > > > > There is indeed a problem with the EFL port. After the enablers in the common code (e.g. like this patch) are landed, the issue with the EFL port will be addressed. > > Wouldn't it be better to address the issue first? It looks to me that the fix for EFL has a dependency on this patch in the shared code. The EFL specific fixes are part of bug 98580. This bug + bug 98580 + bug 98978 resolves the issue for EFL. I figured that this patch is a good place to start the discussion for these serious of patches.
Byungwoo Lee
Comment 8 2012-10-30 21:43:54 PDT
Created attachment 171578 [details] Patch
Byungwoo Lee
Comment 9 2012-10-30 21:49:35 PDT
(In reply to comment #8) > Created an attachment (id=171578) [details] > Patch Rebased the patch and Modified the change log more clearly.
Laszlo Gombos
Comment 10 2012-10-30 21:57:16 PDT
Comment on attachment 171578 [details] Patch Last patch looks good to me with the revised ChangeLog. Thanks, r=me.
Byungwoo Lee
Comment 11 2012-10-30 22:01:20 PDT
Comment on attachment 171578 [details] Patch Thank you for the review :)
WebKit Review Bot
Comment 12 2012-10-31 02:10:58 PDT
Comment on attachment 171578 [details] Patch Clearing flags on attachment: 171578 Committed r133001: <http://trac.webkit.org/changeset/133001>
WebKit Review Bot
Comment 13 2012-10-31 02:11:02 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.