Update comm-central for PLDHashTable changes in bug 1050035

RESOLVED DUPLICATE of bug 1163331

Status

defect
--
critical
RESOLVED DUPLICATE of bug 1163331
5 years ago
3 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
Thunderbird 38.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Bug 1050035 changed PLDHashTable's API in the following ways.

- PLDHashTable now allocates its entry storage lazily. (nsTHashtable and
  friends do too, since they are just layers on top of PLDHashTable.) This is a
  nice space win because about 45% of all created PLDHashTables never get any
  elements inserted into them.

- As a result, PL_DHashTableInit() is now infallible. This is possible because
  the allocation of entry storage now only occurs on table insertion, in
  PL_DHashTableAdd().

- An infallible version of PL_DHashTableAdd() has been added. To use the
  fallible version you need a |fallible_t| argument.

- PLD_NewDHashTable() and PLD_HashTableDestroy() have been removed. You should
  now just use |new|+PL_DHashTableInit and PLD_HashTableDestroy()+|delete|,
  which are more obvious and only slightly more verbose. (And I have plans to
  add a destructor and an initializing constructor, so in the end you'll be
  able to just use |new| + |delete|).
  
I've prepared a patch to get c-c compiling again.
This patch keeps all the PLD_HashTableAdd() calls fallible by adding a
|mozilla::fallible| argument. It might be possible to get rid of
TokenHash::mTableIsInitialized but I didn't do that.

The patch is enough to get c-c compiling. I tested that starting and
immediately shutting down Thunderbird works, but nothing beyond that. A TB dev
should test this and land it. Thanks.
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Component: General → Backend
Product: Thunderbird → MailNews Core
Attachment #8559456 - Flags: review?(kent)
Comment on attachment 8559456 [details] [diff] [review]
Update comm-central for PLDHashTable changes in bug 1050035

Yes this looks fine. Thanks so much for doing this!
Attachment #8559456 - Flags: review?(kent) → review+
Pushed https://hg.mozilla.org/comm-central/rev/1037eb322c0f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 38.0
I had to back out the patches in bug 1050035 due to some intermittent crashes and assertion failures, so this patch will also need backing out. Sorry.
Posted patch Backout patchSplinter Review
Attachment #8562989 - Flags: review?(rkent)
Blocks: 1132218
Comment on attachment 8562989 [details] [diff] [review]
Backout patch

Review of attachment 8562989 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks for doing this!
Attachment #8562989 - Flags: review?(rkent) → review+
Pushed https://hg.mozilla.org/comm-central/rev/0eb00ed42931
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
It looks like the commit fur this bug on 2015-02-05 busted my Thunderbird extension, BiDi Mail UI; see bug 1187572. I see a backout patch on 2015-02-11, but it seems bustage lingered on since February. I only just noticed it a short while ago after TB v38.0 was out. Can you please advise regarding what I/you can/should do about this?
(In reply to Nicholas Nethercote [:njn] from comment #4)
> I had to back out the patches in bug 1050035 due to some intermittent
> crashes and assertion failures, so this patch will also need backing out.
> Sorry.

Can we polish this off?  
Bug 1050035, bug 1144649 and bug 1131901 are long resolved in version 38.
This ended up fixed in a different bug.
Status: REOPENED → RESOLVED
Closed: 5 years ago3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1163331
You need to log in before you can comment on or make changes to this bug.