62084 - Long mailing list does not get imported from ldif file/AB crash

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Address Book & Contacts, defect, P2)

Description

[James Rome]

On build 2000113004:

I exported an ldif file from my address book in 4.76. When I import it, the long
mailing list for stelnews is not there! It is in the ldif file.

This may be a limit on the number of lists imported (stelnews is at the end) or
on the size of the list (its not that big...).

I tried this several times on several different machines.

I will be glad to mail you the ldif file.

Comment 1

[chuang]

Reassign to myself.  Please email me the ldif file.

Comment 2

[Keyser Sose]

Marking NEW to get it off our radar.

Comment 3

[James Rome]

I emailed the ldif file but have heard nothing on this. It still does not work!

Comment 4

[fenella]

QA-assign-to fenella.

Comment 5

[fenella]

James, how large is your ldif file? Can you mail it to [e-mail]? I
want to try it using the newest build.

Comment 6

[James Rome]

Not only do long mailing lists not import, short ones do not work. My list with
just 3 people on it gets rejected by the smtp server because the message gets
sent with the list name instead of the e-mail addresses. This is absolutely a
vital capability.

I have sent my ldif file to many people for this bug, and have never received
any feedback on it.

Comment 7

[James Rome]

My e-mail bounced to fenella...
Here is what I wanted to say:
Did you ever try my LDIF file? I attach it again. Even short mailing lists do
not work (although they import). The Mozilla address book is truly awful
compared to Netscape's original. Let me list the problems:
1) No way to merge address books. You can't even cut and paste. There should at
least be a way to import additional records into an existing address book.
2) No address selection input window that functions like the address line in the
Mozilla mail sender-- namely it finds the addresses in a long list
3) Mailing lists do not work
4) On Windows 2k, there is a general problem about where the address books are
stored. They are put under my user data under documents and settings. The
problem with this is that my user profile gets munged occasionally, and I lose
all my Mozilla personal everything! There needs to be an option to put things in
subdirectories under Mozilla a la Netscape. Anything controlled by a MS
operating system is unreliable!
5) No directory support--still. Well, there is some, but very poor, and not in
the address book. ldap:// works, but ldaps:// does not. It is also very hard
(imnpossible) to figure out the right LDAP root, and entering it into the URL is
a pain.

Comment 8

[James Rome]

This bug seems to get NO attention.
In the latest build 2001102708, I bit the bullet and tried to build my mailing
list. Guess what? When you get more names than can fit in the drag-to box, it
either crashes, or it refuses to open/edit/or use the mailing list. It is
possible that there is also a limit on the number of lists I am hitting. I have 6.

This is a critical bug. It is impossible to really do e-mail if you cannot make
mailing lists with more than 6 names on them!

I was able to finally use a short list.

Comment 9

[James Rome]

I was finally able yo drag names from one address book to another, but NOT
mailing lists.

Now when I try to make even a short list in my default book (fortified by
dragging the names from my imported book into it), when I hit OK, nothing
happens and a list is NOT created.

When one imports addresses, you should be able to choose the book they get
imported into rather than always forming a new book.

Comment 10

[James Rome]

OK. Here is try 3.... What seems to happen is that after about 5 names are
entered on the list, no scroll bar appears. All that one sees is blank fill-in
fields. It is impossible to type in entries, but they can be dragged in.

Clicking on the list in the left-pane (under the expanded address book) does
NOTHING. But double clicking it in the main list brings it up for editing.
However, due to the above behavior, it appears blank and is uneditable!

The address book really needs help....

Comment 11

[James Rome]

Still nothing on this bug. Mailing lists do NOT work at all if they have more
than a few names. The mail does not send. This seems to get no response at all
from anyone. Am I the only one who tries to use a list?

This bug does not show up when I search for mailing lists either.

Comment 12

[Ninoschka Baca]

I am sorry for the delay in this response. You are correct, there are many
problems with the Address Book. Very soon there will be a major change to the
Address Book to use the new Outliner so many of these issues will be fixed but
other problems may be introduced, we'll see. For the long term it should make it
better.  In the meantime these are my results.

Mozilla build 0.9.6: After importing an ldif file which includes a list:
- it successfully displayed the cards and lists. 
- I reproduced the problem with the scrollbar not appearing in a list that was
imported/ldif (this is a known issue w/the 0.9.6 build). 
- I was able to address a new message to the list and it sent/received the
message successfully.

Commercial trunk build 2001-12-18-03: WinMe
- I was able to import an ldif file with cards/lists which imported correctly
- A scroll bar now appears for long lists
- Sucessfully sent/received using the list

Comment 13

[James Rome]

I can't get a list to work with more than about 5 names. And then, I often am
unable to send mail using the list.

Another urgent request is a name picker box. It is really hard to scroll down
through the name list (especially on a laptop's small screen) to find a name.

Also, if a name gets imported in "", it does not even appear in the book!

Let me know how I can test your new version.

Comment 14

[scottputterman]

reassigning to cavin.

Comment 15

[James Rome]

The latest 1/2/02 builds fix lots of sddress book problems. I haven't tried an
import again, but it seems possible to make a kinda mailing list.

However, if during entering the names, you go to the main browser window, and
return the the list entry, it is impossible to get the cursor to appear.

Also, sometimes, the list entry/edit window comes up with no OK button!

Comment 16

[Ninoschka Baca]

James, it would be interesting to know how the ldif file imports and whether
your mailing lists are imported correctly.

Regarding the extra issues you describe then please log these as separate bugs
so the focus of this bug remains on ldif import of mailing lists. 

In the new bugs please list steps so we can try to duplicate the problems. I
tried to reproduce what you describe but couldn't. For instance:
- how many addresses are listed in the Mailing List dialog before you switch to
the Browser?
- When switching do you use Alt+Tab or is the Browser minimized so you use your
mouse and click on the minimized Browser icon?
- When does the OK button disappear?

Comment 17

[James Rome]

I just checked this again on the 01/07/02 build for win2k. The small address
lists imposr, but one about 30 names does not.

Comment 18

[Erik Hensema]

*** Bug 93911 has been marked as a duplicate of this bug. ***

Comment 19

[Dan Meek]

For me, this is a crippling bug which prevents my migration to Mozilla.

Mozilla 0.9.8 does not import address books with any accuracy.  It cannot 
even import small address books without dropping several lists entirely and mis-
naming other lists.  For example, list A will end up populated by the email 
addresses that were on list B in the original Netscape 4.79 addressbook.


This may be same bug as 58206 and 113780 and 50592.

Comment 20

[James Rome]

Editing and adding to lists crashes every time with 2002021403.

I have a list with 19 names. When I drag the 20th name to it and click OK, it
dies every time. Several feedbacks were generated.

Scrolling the list editor also causes it to crash.

Comment 21

[harrij]

I get this with Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.9.8+) Gecko/20020219

Importing NS 4.79 exported LDIF address books loses entries from mailing lists.
The lost entries are always the same ones. Lost entries are imported correctly
as address cards, but they don't show up in the mailing lists. Apparently only
four entries are included to each of the mailing list. This appears to happen
only to address books that are imported separately from Personal and Collected
books eg. MyOtherBook.

I can provide the .ldif file if anyone's interested.

Comment 22

[Michael Buckland]

Discussed in 2/25 mail news bug mtg with Mktng, PjM, and Engineering.  Decision
to  nsbeta1+, change from P3 to P2,  and make milestone 1.0.

Comment 23

[James Rome]

I hope this gets more priority. Life without mailing lists more than 19 names is
impossible for many people!

Comment 24

[Cavin Song]

Reporter and harrij, can you email me your ldif files so that I can make sure 
that your problems will be fixed? Thanks.

Comment 25

[harrij]

Mailed the LDIF file to Cavin.

Comment 26

[Cavin Song]

Got ldif files from reporter and harrij.  harrij's problem is caused by case 
sensitivity issue in the email address field. In other words, if I change 
"[e-mail]" to "[e-mail]" then this member is added to 
the list ok. Working on a fix now and I'll look into reporter's ldif file next.

Comment 27

[Cavin Song]

Reporter's problem is caused by the parsing problem on the input data. The code 
reads 1K data at a time and a single card/contact info may spread into two 
buffers. Each card/contact or list/group info is separated by two consecutive 
CRLFs so if the second CRLF falls into the second buffer then we end up passing 
two card info to a function which only takes care of the first one.  So in this 
case a card is missing and if a list/group also includes this missing card then 
it'll be missing a member as well. I'm working on a fix.

Comment 28

[James Rome]

While fixing things, I noticed with the program Dawn (that imports and exchanges
address books), that it was sensitive to the cases of field names like the CNs.
Be sure none of the infrastructure stuff (i.e., not names and addresses) are
sensitive to case.

Also, do you have a clue as to why Mozilla always crashes (on the save) if I try
to make a manual list with more than 19 names?

Thanks for finally attacking this. My ldif was exported from Netscape 4.7x, so
for sure, the new Netscape should be able to read it!

Comment 29

[Cavin Song]

> Be sure none of the infrastructure stuff (i.e., not names and addresses) are
> sensitive to case.
>
The import program does handle keyword sensitivity issue well.

> Also, do you have a clue as to why Mozilla always crashes (on the save) if I
> try to make a manual list with more than 19 names?
>
Not sure. If it still happens to you with the latest build please file a 
separate bug to track the problem.

Comment 30

[Cavin Song]

The missing lists problem is caused by the fact that we only use a 1K buffer to 
hold the list data so if the data for a particular list is longer than 1K then 
the list is not created. So here is the summary of the ldif import problems:

1. Case sensitivity issue of the email addresses.
2. Parsing problem on the input records when data of the records spread into two 
buffers.
3. Buffer used to hold list data is too small.

The above issues have been resolved and all the test ldif files that were sent 
to me can now be imported correctly.

One thing that needs to be stated here about the ldif import process is that 
when a member is added to a list, the email address is used to see if a card 
associated with this email exists. If no card exists for this email address then 
this member is not added to the list (ie, ignored). For example, if you want to 
add a member whose email address is "[e-mail]" to list "Staff" and no 
card with email address "[e-mail]" already exists yet (in the 
addressbook) then list "Staff" will not contain this member when import 
finishes. I noticed that there are a few members like that in report's ldif file 
(so some lists may miss one or two members).

Comment 31

[James Rome]

Great! I knew there was a reason the list died after a small number of names. I
hope you allowed the buffer to grow as needed. 

The behavior when the card does not exist is correct. If there is no card, the
name should be tossed from the list.

When will this be in a build so that we can test it out?

Comment 32

[Cavin Song]

Attachment: Proposed fix, v1

To fix problem #1 stated above, I made a change to the call to
GetRowFromAttribute() in nsAddrDatabase::AddLdifListMember() (to not convert
email address to lower case when adding members). I did a few tests for normal
addrbook add/edit/delete operations and found that AddLdifListMember() was
never called, so I assume the change is safe. This change also allows us to
retain original case for the member email addresses. Another workaround is to
add code to the import source to convert all email addresses to lower case
beofer creating cards.

Comment 33

[Jörg Thönnes]

Please see comment #6:

My list with just 3 people on it gets rejected by the smtp server because the
message gets sent with the list name instead of the e-mail addresses.

It this issue also addressed by the proposed fixes? Otherwise, see bug 118125
for this problem.

Thanks, Jörg

Comment 34

[Cavin Song]

I'm not 100% sure if the problem described in comment #6 is fixed but I did try 
my own imported list (with 4 members) without problem.

Jörg, if you like, you can email me your ldif file and I'll try it here. Make 
sure that it's ok for me to send test msgs to users on your list though. Or, you 
can wait until after the patch is checked in and then try the new build to see 
if it works for you. 

Comment 35

[Cavin Song]

Attachment: The right patch, v1

Apparently I attached the wrong (not clean) patch. So here it's again.

Comment 36

[(not reading, please use [e-mail] instead)]

cavin and I are discussing the nsAddrDatabase.cpp change, and how it relates to 
another bug (#121478)

as for the import code, looks good, except we malloc and free more than I think 
we need to.

could you keep track of the size of the buffer, and only malloc and free if you 
need to grow the buffer?  or will that make the code much more complicated?  
right now, it's pretty simple and straight forward.



+            // Allocate enough space for the lists/groups as the size varies.
+            listBuf = (char *) PR_Malloc(size);
+            if (!listBuf)
+              continue;
+            if (NS_SUCCEEDED(pSrc->Read(&listBuf, size, &len)) && len > 0)
             {
                 startPos = 0;
 
-                while (NS_SUCCEEDED(GetLdifStringRecord(buf, len, startPos)))
+                while (NS_SUCCEEDED(GetLdifStringRecord(listBuf, len, 
startPos)))
                 {
                     if (m_ldifLine.Find("groupOfNames") != -1)
                     {
@@ -946,6 +953,7 @@
                     }
                 }
             }
+            PR_FREEIF(listBuf);
         

Comment 37

[(not reading, please use [e-mail] instead)]

taking to cavin, we both agree that the import code doesn't need to be made 
more complicated.  keeping this code simple as possible has benefits.

It's not called frequently, this isn't going to have a major impact on 
performance.  (If it does, we can always re-consider.)

sr=sspitzer on the import part, but cavin and I are still discussiong the 
addrbook part.

Comment 38

[Cavin Song]

Attachment: Removed patch to nsAddrDatabase.cpp

Removed patch to Problem #1 as it'll be filed as a separate bug.

Comment 39

[Navin Gupta]

Comment on attachment 72132 [details] [diff] [review]
Removed patch to nsAddrDatabase.cpp

r=naving

Comment 40

[(not reading, please use [e-mail] instead)]

Comment on attachment 72132 [details] [diff] [review]
Removed patch to nsAddrDatabase.cpp

sr=sspitzer

Comment 41

[Cavin Song]

Talked to Seth and we decided to move problem #1 (Case sensitivity issue of the 
email addresses) to a separate bug because we need to investigate this 
addressbook issue fully before committing a change.  In addition, it may also 
affect other related bugs as well.

So, before this "case problem with email addresses" issue is resolved please 
make sure that in the ldif files you use only LOWER CASE email addresses in the 
card/contact info as well as in the list/group members.

Harrij, you'll need to do that in your ldif file because all the missing members 
have upper case in the email address field.

Comment 42

[Cavin Song]

Problem #1 has been moved to bug 128535.

Comment 43

[Asa Dotzler [:asa]]

Comment on attachment 72132 [details] [diff] [review]
Removed patch to nsAddrDatabase.cpp

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 44

[Cavin Song]

Fix checked into trunk.

Comment 45

[Ninoschka Baca]

Trunk build 2002-03-05: WinMe, Linux RH 7.1, Mac 9.1
Verifie Fixed, (only used lowercase characters in email address, had a list of 
30 and 40 addresses in lists)

Comment 46

[Ninoschka Baca]

*** Bug 113780 has been marked as a duplicate of this bug. ***

Comment 47

[James Rome]

This fix seems to have broken the mailing list feature all together. with the
2002030508 build on win2k, if I open a mailing list for editing, when I click
the main address book window  (to get addresses to drag), it beeps and will not
let me change the focus out of the list edit window.

Also, there are lots of blank lines on my existing list that I cannot delete.
The cursor will not rest on the blank lines.

Comment 48

[Cavin Song]

I don't think this is an import bug.  If you manually create a list and then 
open the list for editing do you see the same problem (ie, the Mailing List 
dialog is modal)?

Comment 49

[James Rome]

Manual lists do not work either. But my point is that a fix in the last few days
has changed this behavior, and the import fix is the only one I know of that
might be the culprit. I will file a separate bug, however.

Comment 50

[scottputterman]

Closing. This was done on purpose and isn't related to this bug, see 128124.

Comment 51

[James Rome]

Why oh why would the powers that be make a change that breaks mailing lists all
together?

Despite all the supposed QA that goes on here, I have seen nothing but
regressions lately.

Comment 52

[Cavin Song]

bug 128535 has been fixed so problem #1 stated in Comment #30 is no longer an 
issue. In other words, the following statement in Comment #41 is no longer true:

"
So, before this "case problem with email addresses" issue is resolved please 
make sure that in the ldif files you use only LOWER CASE email addresses in the 
card/contact info as well as in the list/group members.
"

Comment 53

[Mark Shuttleworth]

I just tested an LDIF file with 750+ cards, and two lists, 
and the lists failed to be imported correctly. I'm using
Mozilla 0.9.9 on WinXP.

Unfortunately, the LDIF file contains many confidential
addresses. I'm happy to mail it to a Mozilla developer,
but not to attach it to the bug report. Would it be a
useful extra stress test? I don't know if the problem is
the same as those encountered by the reporter of this bug,
or if my list highlights different issues, but the import
saga isn't quite over yet!

Comment 54

[Cavin Song]

Mark, can you email me the ldif file so that I can debug the problem? Thanks. I
assume that you're running the build after 03/04/2002.

Comment 55

[Mirek]

Build 2002031104, WinNT4SP6a. Well, I have tried to change all UPPERCASE into
lowercase as recommended in the comment #52, no good. I have also erased all
undescores in the names - again no good. But Netscape 4.79 using the very same
.ldif file imported everything OK.... Could it be just becouse my file contains
also lists of lists? To mee it does not seem fixed/resolved at all. Nothing has
changed. Or, this bug is not duplicate of the original problem I posted as bug
93911 .

Comment 56

[Cavin Song]

Mirek, is it possible for you to email me your ldif file so that I can take a
look at it? The fix has worked for 5 (reported) users' lidf files so I'm curious
about your problem. Thanks.

Comment 57

[Cavin Song]

I see what the problem is. In your ldif file member entries are missing email 
addresses. For example,

 member: cn=Bogomolova Anna, o=CERGE-EI

should have been:

 member: cn=Bogomolova Anna, o=CERGE-EI, mail=[e-mail]

The reason is that email addresses are keys to locate the addressbook 
cards/contacts and if  the cards/contacts for the members can't be found then 
members won't be added to the list. This is the way ldif import currently works.

I think we should close this bug and file a separate bug to allow members to be 
imported using only the 'cn' attribute values.

Comment 58

[Cavin Song]

FYI, Mark Shuttleworth's problem is resolved with a recent build.

Comment 59

[Ninoschka Baca]

Verified Fixed.

Comment 60

[Dan Meek]

I humbly submit that this bug is not fixed.  I have tried to import my LDIF 
file, created with Netscape 4.79, using the latest build (3/22).  The result of 
the attempt to import is that Mozilla crashes and unloads itself entirely.  I 
have provided the offending LDIF file to [e-mail], who said yesterday 
"I still can't make it happen on both NT and 2K."  It is not clear whether he 
has been able to import my LDIF file.

Comment 61

[Cavin Song]

Yes, I have been trying to reproduce Dan Meek's or the past two days and I 
failed to do so. I have tried NS and Mozilla builds on both NT an 2K 
machines. So Dan Meek, did you get a chance to log something to talkback? Maybe 
I can take a look at the stack trace if you did.

Comment 62

[Dan Meek]

The latest is that build 2002-03-30 also crashes when attempting to import one 
of my LDIF files.  Even with talkback enabled, the crash is clean and does not 
offer the opportunity to send a report.

I tried importing a shorter LDIF.  Mozilla did not crash but also failed to 
recreate any of the lists contained in the LDIF file.

On a different computer, build 2002-03-30 also crashed but produced this error 
message:

MOZILLA caused an invalid page fault in
module MORK.DLL at 018f:6074793c.
Registers:
EAX=0000b561 CS=018f EIP=6074793c EFLGS=00010202
EBX=017ddfa0 SS=0197 ESP=0064e10c EBP=0064e148
ECX=0196425c DS=0197 ESI=019641f8 FS=4b47
EDX=0196410c ES=0197 EDI=00000070 GS=42ef
Bytes at CS:EIP:
8b 10 89 11 eb 13 8d 47 04 8b ce 01 46 24 50 53
Stack dump:
00000000 00000070 0000000d 607465ac 017ddfa0 00000070 0000000d 04ba212c
6074660e 017ddfa0 0000000e 019641f8 04ba212c 00000001 01964644 0064e16c

Comment 63

[Dan Meek]

I figured out that I have to manually run talkback.exe and turn it on, which I 
did.  Mozilla again crashed when attempting to import my long LDIF file 
(meek2.ldif) and produced a talkback screen.  I saved the report (342k) and sent 
it to Cavin.

Also, about the smaller LDIF that imported without the lists:  After Mozilla 
unloaded and I reloaded it, the lists for that imported LDIF suddenly appeared. 
 They most definitely were not there, immediately upon their import into the 
Mozilla address book.

Comment 64

[Asa Dotzler [:asa]]

Dan Meek, I'm not sure that Cavin can do anything with the talkback data you
sent him. It is my understanding that the data needs to be processed which
reconnects it to the necessary symbols and whatnot giving a meaningful stack
trace. Did you submit the report from the talkback client? If not, can you? If
you attach the TalkBack ID (running the talkback.exe should load the client and
show you all the incidents that is has recorded and give you an ID for any that
have been sent into the talkback servers) I can look it up and paste a
stacktrace into this bug. 

Cavin, if it's not too disruptive to your process can you please obsolete the
latest patch in this bug so it doesn't come up on driver's radar as an approved
patch waiting to be checked in? If it does mess up your queries or something
like that then don't worry, we'll just work around this one. Thanks.

Comment 65

[Dan Meek]

I did send the traces from the 2 crashes.  Their IDs are TB4658369W and 
TB4657538X.

Comment 66

[Cavin Song]

Thanks for the ids. The stack trace is following and I'll pull a new tree to
debug the problem tomorrow:

morkZone::ZoneNewRun [d:\builds\seamonkey\mozilla\db\mork\src\morkZone.cpp, line
390] 
morkPool::NewCells [d:\builds\seamonkey\mozilla\db\mork\src\morkPool.cpp, line 284] 
morkPool::AddRowCells [d:\builds\seamonkey\mozilla\db\mork\src\morkPool.cpp,
line 323] 
morkRow::NewCell [d:\builds\seamonkey\mozilla\db\mork\src\morkRow.cpp, line 456] 
morkRow::AddColumn [d:\builds\seamonkey\mozilla\db\mork\src\morkRow.cpp, line 888] 
morkRowObject::AddColumn
[d:\builds\seamonkey\mozilla\db\mork\src\morkRowObject.cpp, line 270] 
nsAddrDatabase::AddIntColumn
[d:\builds\seamonkey\mozilla\mailnews\addrbook\src\nsAddrDatabase.cpp, line 2126] 
nsAddrDatabase::AddLdifListMember
[d:\builds\seamonkey\mozilla\mailnews\addrbook\src\nsAddrDatabase.cpp, line 2060] 

Comment 67

[Mark Shuttleworth]

In my case, cavin could not reproduce the problem, so I tried 
with a brand new profile and a post-0.9.9 nightly, and voila, 
everything worked perfectly. Dan Meek, are you using a profile
that has been around through many nightly builds, or a fresh 
one? BTW, thanks to cavin for helping me out so quickly.

Comment 68

[Dan Meek]

Cavin a few days ago suggested using a new profile, which I did.  My email to 
him on March 31, 3am, stated:

     I just created a new profile named test.
     I tried to import the meek2.ldif address book.
     Result was complete crash, with attached error file.

I am pretty sure that the error file I attached was TB4657538X.

Thus, for me, creating a new profile and using the 20020330 build did not work. 
 Instead, it crashed Mozilla entirely.

Comment 69

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 70

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 71

[David :Bienvenu]

I can't find either a talkback report with that id (4657538X) or one with the
email address [e-mail] so I'm not sure if the current crash is the same
stack trace. Without seeing the ldif file, I'm at a bit of a loss to recreate
this. I understand Cavin can't recreate it either with the .ldif file in
question but I'd be happy to try if Dan would send it to me. How many entries
are in the ldif file? > 64,000 entries could cause a problem, but I don't know
of any other existing problems. There was a problem in this area fixed back in
January but I believe the crash attached is from a build with that fix in it.

Comment 72

[Dan Meek]

I have emailed a file that does not import to  [e-mail].  It has 
maybe 4000 entries.

Comment 73

[Dan Meek]

Also, I believe that Cavin has not been successful in importing this same file. 
 I asked him to send me the correctly imported addressbook, if he was able to 
succeed.  I have not received one.  Thus, I believe that he has reproduced the 
problem.  

Comment 74

[David :Bienvenu]

Cavin had indicated privately to me that he was not able to reproduce this
problem, but I am able to recreate it myself. That's the good news. The bad news
is that it might take me a while to fix this since these Mork bugs can be quite
hard. taking from Cavin.

Comment 75

[Cavin Song]

David was right as I have not been able to recreate the problem. It's good that 
David is now able to see the problem. Thanks for taking the bug.

Comment 76

[David :Bienvenu]

I figured out some of what's going on - when mork allocates new cells for a row,
it uses morkPool::AddRowCells, which uses a zone allocation to get the memory
for the new cells. morkZone::ZoneNewRun(mdb_size inSize) finds the right zone
for the new block of memory (by dividing by 16) and then returns a free block
from the zone. Zones basically have a bunch of buckets, each bucket
corresponding to a different memory block size. The buckets in turn are linked
lists of free blocks. What's happening is that the block of memory we're getting
is coming from the buckets for blocks of size 64-127, but it seems like the free
block is really only 16bytes, because there's another block that starts 16 bytes
after the free block we're returning. Clearing out the new block crunches the
other block. The other block happens to be the old block of cells, so we crunch
the old block and crash trying to copy the old cells to the new cells. That's as
far as I've gotten; I don't know why the block is incorrect, and because of the
custom allocators, I can't use Purify to determine what's going on.

Comment 77

[David :Bienvenu]

locally, I switched mork not to use zones and this problem goes away. I'm not
sure if that means there's a bug with zones, or just that a different piece of
memory is getting crunched somehow and doesn't cause a noticeable problem. (we
use zones for performance reasons so not using them just to fix this bug is
probably not an option). I'll try running under purify with my local change to
see if it catches any problems now that we're using the normal allocator for cells.

Comment 78

[David :Bienvenu]

the problem is with zones. When we request a memory allocation that's larger
than the zones we keep track of with buckets (> 4096 bytes, which means 256
columns), then we allocate a morkOldRun, but when we free the run, we don't
treat it correctly as a morkOldRun, and we "run" into various problems. I've
tried tweaking the code in a bunch of ways, but there's so much code that
assumes morkRuns and morkOldRuns are the same, that it's very difficult.

Comment 79

[David :Bienvenu]

OK, I finally have a fix for this. Now I need to go back and figure out if all
the other changes I made are really needed, or if the final changes I made are
the crucial ones.

Comment 80

[David :Bienvenu]

Attachment: proposed fix

there were two problems here, in morkZone::zone_grow_at

1. When removing the first element of the linked list, we were not setting the
head pointer to the second element of the list.
2. When we found a useable old run, we were setting the free pointer to the
runAsBlock, and not the run itself (the block starts after the housekeeping
info in the run) - this was wrong, because we want the free pointer to be the
whole run, and also, the runSize includes run info, so we ended up overshooting
the end of the block, which was causing the problem.

Comment 81

[David :Bienvenu]

Navin, can I get a review? thx.

Comment 82

[Navin Gupta]

Comment on attachment 82754 [details] [diff] [review]
proposed fix

r=naving, very cryptic, don't know much about this.
if you can get someone else
to review also, it would be good.

Comment 83

[Cavin Song]

*** Bug 50592 has been marked as a duplicate of this bug. ***

Comment 84

[(not reading, please use [e-mail] instead)]

Comment on attachment 82754 [details] [diff] [review]
proposed fix

sr=sspitzer

Comment 85

[David :Bienvenu]

fix checked in.

Comment 86

[David :Bienvenu]

I'm going to renominate - this is a fundamental little flaw in the mork memory
allocation code, and shows up pretty consistently in the talkback logs. I think
if you were a heavy addressbook user, you'd see this crash frequently.

Comment 87

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) for approval to checkin to the 1.0 branch, pending
Drivers' approval.  After, checking in, please add the fixed1.0 keyword.

Comment 88

[Scott Collins]

Comment on attachment 82754 [details] [diff] [review]
proposed fix

a=scc (on behalf of drivers) for checkin to the mozilla1.0 branch

Comment 89

[David :Bienvenu]

fix checked into branch.

Comment 90

[Ninoschka Baca]

Branch and Trunk builds 2002-05-28: WinMe
Verified Fixed.