8646 - Use of stringbundle on netlib thread causes crash in viewer

Status: VERIFIED WORKSFORME

(SeaMonkey :: Passwords & Permissions, defect, P1)

Description

[buster]

I'm just guessing this is signon code at fault.
go to any page with a form that has a password field, for example

http://edit.my.yahoo.com/config/login?.src=quote&.done=http://finance.yahoo.com/
?u

enter a user name and password and click "sign in"
If you break into the infinite loop that results, you get a stack like this:
NTDLL! 77f681ab()
KERNEL32! 77f04f85()
_PR_WaitCondVar() line 175 + 23 bytes
PR_Wait() line 155 + 29 bytes
nsBlockingStream::Read() line 718 + 14 bytes
ByteBufferImpl::Fill() line 127 + 36 bytes
ConverterInputStream::Fill() line 266 + 33 bytes
ConverterInputStream::Read() line 239 + 12 bytes
nsPersistentProperties::Read() line 403 + 30 bytes
nsPersistentProperties::Load() line 227 + 8 bytes
nsStringBundle::nsStringBundle() line 163 + 22 bytes
nsStringBundleService::CreateBundle() line 234 + 39 bytes
Wallet_Localize() line 401 + 24 bytes
Wallet_SetKey() line 766 + 10 bytes
si_SetKey() line 300 + 7 bytes
si_SaveSignonDataLocked() line 2045 + 5 bytes
si_PutData() line 1396 + 7 bytes
SINGSIGN_RememberSignonData() line 2231 + 15 bytes
nsWalletlibService::SI_RememberSignonData() line 133 + 25 bytes
nsFormFrame::ProcessAsURLEncoded() line 884 + 53 bytes
nsFormFrame::OnSubmit() line 476
nsButtonControlFrame::MouseClicked() line 353
nsFormControlFrame::HandleEvent() line 605
nsButtonControlFrame::HandleEvent() line 533
PresShell::HandleEvent() line 2043 + 38 bytes
nsView::HandleEvent() line 833
nsView::HandleEvent() line 818
nsView::HandleEvent() line 818
nsView::HandleEvent() line 818
nsView::HandleEvent() line 818
nsViewManager::DispatchEvent() line 1735
HandleEvent() line 67
nsWindow::DispatchEvent() line 408 + 10 bytes
nsWindow::DispatchWindowEvent() line 429
nsWindow::DispatchMouseEvent() line 3007 + 15 bytes
nsWindow::ProcessMessage() line 2369 + 24 bytes
nsWindow::WindowProc() line 471 + 27 bytes
USER32! 77e71250()

Comment 1

[buster]

I don't know what we're marking dogfood blockers, but I added the keyword
DOGFOOD.  You get the point, we can't advertise this as a real browser with this
bug.  Marked "blocker" with P1 priority.

Comment 2

[Stephen P. Morse]

What build are you running?  I can't reproduce this problem at all -- it works
fine for me.  I couldn't find any password field on the url you suggested
(quote.yahoo.com) so I went to scopus/bugsplat instead.

From the stack trace you are reporting, it looks like a problem reading the
wallet.properties file needed for i18n.  So I'm copying Erik van der Poel to see
if he has any ideas.  But unless someone can figure out what I am doing
differenly from buster who reported the bug, I'm going to have to close this out
as works-for-me.  Paulmac, are you able to reproduce this?

Comment 3

[Erik van der Poel]

We have a problem in StringBundles due to the old netlib not working under
some sort of thread-related circumstances. This one may be related.

Comment 4

[buster]

More info:
This does not seem to happen in apprunner, only viewer.  This is happening in
viewer on WinNT on 2 different machines, using code from midnight 6/20/99 on one
and noon 6/21/99 on the other.  Both are debug builds.  Since it's happening on
2 different machines, I really think you should reconsider marking it worksforme
without further research.  Sure, it could be a configuration issue, but I have
standard installations built from scratch.
As for finding the password field, I added the full URL from the original
description into the URL field of the bug report.

Comment 5

[Stephen P. Morse]

Thanks, Buster, now I know why it worked for me and not for you.  I was using
apprunner of course.  Are we supporting viewer?  Does Q/A do any testing on
viewer?

In any event, the bug is certainly below wallet and involves the nsStringBundle
routine.  From Erik's comments, he seems to have more of a handle on it so I'm
reassigning the bug to him.

Comment 6

[buster]

Yes, viewer is supported.  It's an important test bed for many people.  There is
also value in making sure our components are playing fair by assembling them in
more than one app.

Comment 7

[Paul MacQuiddy]

For what it's worth, QA does not do any of it's testing on viewer. Leger would
be the one to talk to if there is a misunderstanding on that.

Comment 8

[Erik van der Poel]

I'm not going to fix StringBundle until Necko is stable, so please disable
your use of it in Wallet.

Comment 9

[Stephen P. Morse]

Well if this bug is being reassigned to me (for a second time), then I have no
choice but to mark it as won't fix.  I'm not going to disable use of
nsStringBundles in wallet because it is working fine in apprunner (recall this
is a viewer-only bug).

Comment 10

[buster]

I think it's an extrodinarily bad idea to mark this "won't fix" in the hopes
that Necko will just make it work.  I'm OK with not trying to fix it until Necko
lands (setting the milestone for M9 or M10.)  But the bug should certainly
remain open, and seems like it should be assigned to Erik if everybody agrees
it's a bug that is primarily revealed by StringBundle's use of netlib.  Then
Erik can either mark it fixed if Necko saves the day, or this can act as a
reminder to fix whatever is really broken.

Comment 11

[Erik van der Poel]

It is not immediately clear to me that this bug is due to StringBundle's use of
netlib. StringBundle takes a URL as an argument, and calls OpenBlockingStream.
I think it needs to be a blocking stream, since we probably don't want to have
an asynchronous interface for this. If we can agree on this, then Wallet should
probably call StringBundle on a thread other than the "UI" thread.

On the other hand, if we believe that StringBundles should never be remote, so
that we can call a synchronous local file read function, then we may need to
alter the design of StringBundle itself. Comments?

Comment 12

[Stephen P. Morse]

Marking as M9 since, according to erik, this depends on necko.

Comment 13

[Stephen P. Morse]

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

Comment 14

[Stephen P. Morse]

Changing summary to be more indicative of what the bug is.

Comment 15

[Stephen P. Morse]

Downgrading the severity from "blocked" to "normal" since this affects viewer
only -- apprunner is unaffected.

Comment 16

[Stephen P. Morse]

I just tried this using a necko build of viewer.  I did not get the crash.  I
did get an assertion failure (debug build of course) but I was able to
successfully resume running from that point.

So our expectations that necko would fix this were valid.  Closing this out as
works-for-me.

Comment 17

[buster]

assuming you submitted another bug about the assertion, this looks fine

Comment 18

[Stephen P. Morse]

Assertion failure's aren't necessarily bugs.

Comment 19

[Paul MacQuiddy]

marking verified per buster's comments. You can fight it out about whether to
file a bug on the assert :-)