Bug 30713 - Disable ReadWriteLock on platforms using pthreads but which do not support pthread_rwlock
Summary: Disable ReadWriteLock on platforms using pthreads but which do not support pt...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Steve Block
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-23 06:38 PDT by Steve Block
Modified: 2009-10-26 03:47 PDT (History)
4 users (show)

See Also:


Attachments
Patch 1 for Bug 30713 (3.22 KB, patch)
2009-10-23 06:45 PDT, Steve Block
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2009-10-23 06:38:28 PDT
ReadWriteLock is currently used only on SnowLeopard. See http://trac.webkit.org/changeset/44651. It is therefore implemented only on platforms using pthreads.

However, some platforms that use pthreads (eg Android) do not support pthread_rwlock. For these platforms, ReadWriteLock should be disabled.
Comment 1 Steve Block 2009-10-23 06:45:01 PDT
Created attachment 41723 [details]
Patch 1 for Bug 30713
Comment 2 Eric Seidel (no email) 2009-10-23 11:42:46 PDT
Comment on attachment 41723 [details]
Patch 1 for Bug 30713

This looks sane to me, but whoever added the ReadWriteLock stuff should really review/comment here.
Comment 3 Mark Rowe (bdash) 2009-10-24 00:54:10 PDT
It’s not used anywhere.  I suspect it was added for a short-lived experiment, and that we can now rip it out completely.
Comment 4 Darin Adler 2009-10-25 16:18:28 PDT
Comment on attachment 41723 [details]
Patch 1 for Bug 30713

Seems OK.

I'm not sure it's good to have all these cases, this one included, where the type PlatformReadWriteLock defined to be void*. If we're going to omit ReadWriteLock, then I suggest we do that in the header too rather than compiling a useless ReadWriteLock.

What's more, I can't find any code using ReadWriteLock anywhere in the WebKit source code nor in Safari's source code. It looks like Hyatt added this class but never started using it.

This patch is fine, but I think we could also consider dropping this unused code.

I see that Mark Rowe said the same thing in his recent comment.
Comment 5 Steve Block 2009-10-26 03:26:23 PDT
> What's more, I can't find any code using ReadWriteLock anywhere in the WebKit
> source code nor in Safari's source code.
I wasn't sure if Hyatt was still working on the code that makes use of this class.

I'll commit this patch, but if Hyatt can confirm that the class isn't needed, I'll happily send a patch to remove it completely.
Comment 6 WebKit Commit Bot 2009-10-26 03:47:04 PDT
Comment on attachment 41723 [details]
Patch 1 for Bug 30713

Clearing flags on attachment: 41723

Committed r50052: <http://trac.webkit.org/changeset/50052>
Comment 7 WebKit Commit Bot 2009-10-26 03:47:11 PDT
All reviewed patches have been landed.  Closing bug.