Closed Bug 8646 Opened 25 years ago Closed 25 years ago

Use of stringbundle on netlib thread causes crash in viewer

Categories

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

Product:
SeaMonkey
Component:
Passwords & Permissions
Platform:
x86
Windows NT
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED WORKSFORME

People

(Reporter: buster, Assigned: morse)

References

(
URL
)

Details

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()
Severity: normal → blocker
Priority: P3 → P1
Summary: infinite loop in single signon code → [DOGFOOD] infinite loop in single signon code
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.
Status: NEW → ASSIGNED
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?
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.
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.
Assignee: morse → erik
Status: ASSIGNED → NEW
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.
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.
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.
Assignee: erik → morse
I'm not going to fix StringBundle until Necko is stable, so please disable
your use of it in Wallet.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → WONTFIX
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).
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.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
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?
Status: REOPENED → ASSIGNED
Target Milestone: M9
Marking as M9 since, according to erik, this depends on necko.
*** Bug 9038 has been marked as a duplicate of this bug. ***
Summary: [DOGFOOD] infinite loop in single signon code → [DOGFOOD] Use of stringbundle on netlib thread causes crash in viewer
Changing summary to be more indicative of what the bug is.
Summary: [DOGFOOD] Use of stringbundle on netlib thread causes crash in viewer → Use of stringbundle on netlib thread causes crash in viewer
Severity: blocker → normal
Downgrading the severity from "blocked" to "normal" since this affects viewer
only -- apprunner is unaffected.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → WORKSFORME
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.
assuming you submitted another bug about the assertion, this looks fine
Assertion failure's aren't necessarily bugs.
Status: RESOLVED → VERIFIED
marking verified per buster's comments. You can fight it out about whether to
file a bug on the assert :-)
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.