Bug 226152

Summary:

Use CheckedLock in even more places

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebCore Misc.

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, darin, ddkilzer, eric.carlson, ews-watchlist, ggaren, glenn, hi, hta, jer.noble, joepeck, keith_miller, kkinnunen, mark.lam, msaboff, philipj, saam, sam, sergio, tommyw, tzagallo, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

226157

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	ews-feeder: commit-queue-
Patch	ews-feeder: commit-queue-
Patch	none

Description Chris Dumez 2021-05-22 21:37:08 PDT

Use CheckedCondition in even more places to benefit from Clang Thread Safety Analysis.

Comment 1 Chris Dumez 2021-05-22 21:39:48 PDT

Created attachment 429454 [details]
Patch

Comment 2 Chris Dumez 2021-05-22 21:42:02 PDT

Created attachment 429455 [details]
Patch

Comment 3 Darin Adler 2021-05-23 12:46:22 PDT

Comment on attachment 429455 [details]
Patch

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

> Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:373
> +// FIXME: This modifies m_data without grabbing m_dataLock.

Hard to fix?

> Source/WebCore/worklets/PaintWorkletGlobalScope.h:68
> +    HashMap<String, std::unique_ptr<PaintDefinition>>& paintDefinitionMap() WTF_REQUIRES_LOCK(m_paintDefinitionLock)
> +    {
> +        ASSERT(m_paintDefinitionLock.isLocked());
> +        return m_paintDefinitionMap;
> +    }

Can we put this function body after the class definition to keep the class definition easy to read?

Comment 4 Chris Dumez 2021-05-23 13:00:28 PDT

(In reply to Darin Adler from comment #3)
> Comment on attachment 429455 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=429455&action=review
> 
> > Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:373
> > +// FIXME: This modifies m_data without grabbing m_dataLock.
> 
> Hard to fix?

I will check in a follow-up. I decided to stay away from fixes in my large refactoring fixes based on earlier feedback.

> 
> > Source/WebCore/worklets/PaintWorkletGlobalScope.h:68
> > +    HashMap<String, std::unique_ptr<PaintDefinition>>& paintDefinitionMap() WTF_REQUIRES_LOCK(m_paintDefinitionLock)
> > +    {
> > +        ASSERT(m_paintDefinitionLock.isLocked());
> > +        return m_paintDefinitionMap;
> > +    }
> 
> Can we put this function body after the class definition to keep the class
> definition easy to read?

Will fix.

Comment 5 Chris Dumez 2021-05-23 13:03:18 PDT

Created attachment 429471 [details]
Patch

Comment 6 Chris Dumez 2021-05-23 13:12:05 PDT

Created attachment 429474 [details]
Patch

Comment 7 Chris Dumez 2021-05-23 13:18:07 PDT

Created attachment 429475 [details]
Patch

Comment 8 Chris Dumez 2021-05-23 13:33:36 PDT

Comment on attachment 429475 [details]
Patch

Clearing flags on attachment: 429475

Committed r277934 (238064@main): <https://commits.webkit.org/238064@main>

Comment 9 Chris Dumez 2021-05-23 13:33:40 PDT

All reviewed patches have been landed.  Closing bug.

Comment 10 Radar WebKit Bug Importer 2021-05-23 13:34:17 PDT

<rdar://problem/78372445>