What is PLDHashTable used for?

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

What is PLDHashTable used for?

Matt Morehouse
Hi everyone,


While doing some random testing of Firefox, I encountered the assertion failure MOZ_ASSERT(IsIdle(oldState) || IsRead(oldState)) in Checker::StartReadOp() in xpcom/glue/pldhash.h.  It seems that this failure is the result of concurrent readers and writers to the same instance of PLDHashTable.


It seemed odd to me that there would be code to check for concurrent readers and writers but no code to prevent such concurrent accesses, so I decided to add some basic synchronization with a pthread_rwlock_t.  While this approach did fix the assertion failure, it also slowed Firefox down to a crawl.  Is this the reason that PLDHashTable is currently left unsynchronized?  What is PLDHashTable being used for that requires so many concurrent accesses?


Thank you,

Matt Morehouse
_______________________________________________
dev-tech-xpcom mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xpcom
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What is PLDHashTable used for?

Boris Zbarsky
On 11/18/15 6:22 PM, Matt Morehouse wrote:
> While doing some random testing of Firefox, I encountered the assertion failure MOZ_ASSERT(IsIdle(oldState) || IsRead(oldState)) in Checker::StartReadOp() in xpcom/glue/pldhash.h.  It seems that this failure is the result of concurrent readers and writers to the same instance of PLDHashTable.

Or, possibly, reentry.  What did the stack look like?

> It seemed odd to me that there would be code to check for concurrent readers and writers but no code to prevent such concurrent accesses

PLDHashTable is single-threaded.  It's not meant to be used in any sort
of concurrent way at all.  This code is checking for reentry, really.

> so I decided to add some basic synchronization with a pthread_rwlock_t.

Note that in the reentry case this would probably deadlock....

> While this approach did fix the assertion failure

Sounds like you were actually getting concurrent access, then.  I'd
_really_ like to see the stack at that assertion failure.

> Is this the reason that PLDHashTable is currently left unsynchronized?

Well, that, and that it's not meant to be used in any sort of concurrent
manner that involves thread.

> What is PLDHashTable being used for that requires so many concurrent accesses?

It's the main hashtable implementation used outside the JS engine.  Used
for all sorts of stuff in the DOM, layout, and so forth.

-Boris
_______________________________________________
dev-tech-xpcom mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xpcom
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What is PLDHashTable used for?

Matt Morehouse
Hi Boris,

The backtrace is attached.  The assertion failure occurs at #6.  Doesn't look like reentry to me.

- Matt

________________________________________
From: dev-tech-xpcom <dev-tech-xpcom-bounces+mm=[hidden email]> on behalf of Boris Zbarsky <[hidden email]>
Sent: Wednesday, November 18, 2015 9:54 PM
To: [hidden email]
Subject: Re: What is PLDHashTable used for?

On 11/18/15 6:22 PM, Matt Morehouse wrote:
> While doing some random testing of Firefox, I encountered the assertion failure MOZ_ASSERT(IsIdle(oldState) || IsRead(oldState)) in Checker::StartReadOp() in xpcom/glue/pldhash.h.  It seems that this failure is the result of concurrent readers and writers to the same instance of PLDHashTable.

Or, possibly, reentry.  What did the stack look like?

> It seemed odd to me that there would be code to check for concurrent readers and writers but no code to prevent such concurrent accesses

PLDHashTable is single-threaded.  It's not meant to be used in any sort
of concurrent way at all.  This code is checking for reentry, really.

> so I decided to add some basic synchronization with a pthread_rwlock_t.

Note that in the reentry case this would probably deadlock....

> While this approach did fix the assertion failure

Sounds like you were actually getting concurrent access, then.  I'd
_really_ like to see the stack at that assertion failure.

> Is this the reason that PLDHashTable is currently left unsynchronized?

Well, that, and that it's not meant to be used in any sort of concurrent
manner that involves thread.

> What is PLDHashTable being used for that requires so many concurrent accesses?

It's the main hashtable implementation used outside the JS engine.  Used
for all sorts of stuff in the DOM, layout, and so forth.

-Boris
_______________________________________________
dev-tech-xpcom mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xpcom

_______________________________________________
dev-tech-xpcom mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xpcom

pldhash-stack.txt (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What is PLDHashTable used for?

Boris Zbarsky
In reply to this post by Boris Zbarsky
On 11/19/15 9:27 AM, Matt Morehouse wrote:
> The backtrace is attached.  The assertion failure occurs at #6.  Doesn't look like reentry to me.

Indeed, it doesn't.

This particular hashtable (XPCWrappedNativeScope::mWrappedNativeMap)
should _certainly_ not be touched from multiple threads.  If that's
happening, something is very wrong...

-Boris
_______________________________________________
dev-tech-xpcom mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xpcom
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What is PLDHashTable used for?

olli.pettay
In reply to this post by Boris Zbarsky
On Thursday, November 19, 2015 at 4:28:31 PM UTC+2, Matt Morehouse wrote:
> Hi Boris,
>
> The backtrace is attached.  The assertion failure occurs at #6.  Doesn't look like reentry to me.



And that looks like main thread which is accessing the hashtable, which is expected.
Do you have steps to reproduce? Did you have some addons installed when testing?
_______________________________________________
dev-tech-xpcom mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xpcom
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What is PLDHashTable used for?

Matt Morehouse
Hi again everyone,

I just figured out the real issue, and it was completely my fault.

My testing technique involved modifying the pthreads library to induce unusual thread scheduling patterns.  One modification I made was to keep track of all running threads in a dynamically-allocated data structure.  Turns out concurrent calls to malloc between Firefox and my modified library were causing some memory chunks to be assigned to both Firefox and my data structure, thereby causing data corruption.  After switching to a statically-allocated data structure, the assertion failure went away.

Thank you for your help Boris and Olli, and sorry for wasting your time.

- Matt

________________________________________
From: dev-tech-xpcom <dev-tech-xpcom-bounces+mm=[hidden email]> on behalf of [hidden email] <[hidden email]>
Sent: Thursday, November 19, 2015 8:58 AM
To: [hidden email]
Subject: Re: What is PLDHashTable used for?

On Thursday, November 19, 2015 at 4:28:31 PM UTC+2, Matt Morehouse wrote:
> Hi Boris,
>
> The backtrace is attached.  The assertion failure occurs at #6.  Doesn't look like reentry to me.



And that looks like main thread which is accessing the hashtable, which is expected.
Do you have steps to reproduce? Did you have some addons installed when testing?
_______________________________________________
dev-tech-xpcom mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xpcom
_______________________________________________
dev-tech-xpcom mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-xpcom
Loading...