Closed Bug 1673708 Opened 2 years ago Closed 2 years ago

Crash in [@ InvalidArrayIndex_CRASH | nsPrinterWin::PaperList]

Categories

(Core :: Printing: Setup, defect, P2)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox82 --- wontfix
firefox83 --- wontfix
firefox84 --- fixed

People

(Reporter: aryx, Assigned: bobowen)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

First observed with 82.0b2 but not with 82.0a1 (in general only only nightly and aurora).

Crash report: https://crash-stats.mozilla.org/report/index/a9212254-d85c-43cb-bf52-abb520201027

MOZ_CRASH Reason: ElementAt(aIndex = 71, aLength = 2)

Top 10 frames of crashing thread:

0 xul.dll InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:27
1 xul.dll nsPrinterWin::PaperList const widget/windows/nsPrinterWin.cpp:296
2 xul.dll static std::_C__Invoker_functor::_Call<`lambda at /builds/worker/checkouts/gecko/widget/PrintBackgroundTask.h:54:17'> 
3 xul.dll mozilla::SpawnPrintBackgroundTask<nsPrinterBase, nsTArray<mozilla::PaperInfo>>::<unnamed-tag>::operator const widget/PrintBackgroundTask.h:53
4 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/widget/PrintBackgroundTask.h:48:11'>::Run xpcom/threads/nsThreadUtils.h:577
5 xul.dll nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:299
6 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1197
7 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:332
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:327
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:309

Bob and Jonathan, you touched this code (nsPrinterWin::PaperList / GetDeviceCapabilityArray) somewhat recently, can you take a look please?

Severity: -- → S2
Flags: needinfo?(jfkthame)
Flags: needinfo?(bobowencode)
Priority: -- → P2

Bob, it looks to me like this started shortly after bug 1664981 landed, although I haven't spotted exactly what the problem would be.... any ideas?

Flags: needinfo?(jfkthame)

(In reply to Jonathan Kew (:jfkthame) from comment #2)

Bob, it looks to me like this started shortly after bug 1664981 landed, although I haven't spotted exactly what the problem would be.... any ideas?

This appears to be from bug 1665706.
In the two dumps I looked at where I could see more detail, it looks like for paperIds [1] is returning a higher figure (71 and 43 in the dumps) and then [2] is returning a much smaller one (1 and 2 in the dumps).
We then use that for the count for paperNames, but at [2] it is returning the original higher number and so [3] crashes.

We could try and always use the originally returned count, instead of the final length of paperIds, for subsequent counts.

[1] https://searchfox.org/mozilla-central/rev/a147181ece866c1ecd176ac49f112785f960aac0/widget/windows/nsPrinterWin.cpp#166-167
[2] https://searchfox.org/mozilla-central/rev/a147181ece866c1ecd176ac49f112785f960aac0/widget/windows/nsPrinterWin.cpp#176-178
[3] https://searchfox.org/mozilla-central/rev/a147181ece866c1ecd176ac49f112785f960aac0/widget/windows/nsPrinterWin.cpp#185

Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
Flags: needinfo?(bobowencode)
Regressed by: 1665706
Has Regression Range: --- → yes
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/aac7d9a5298c
Use the original required length for paperIDs in nsPrinterWin::PaperList. r=jfkthame

Set release status flags based on info from the regressing bug 1665706

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

Comment on attachment 9184914 [details]
Bug 1673708: Use the original required length for paperIDs in nsPrinterWin::PaperList. r=jfkthame!

Beta/Release Uplift Approval Request

  • User impact if declined: Small number of users will experience browser crashes when trying to print.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Relatively simple changes to single source file.
  • String changes made/needed: None
Attachment #9184914 - Flags: approval-mozilla-beta?

Comment on attachment 9184914 [details]
Bug 1673708: Use the original required length for paperIDs in nsPrinterWin::PaperList. r=jfkthame!

11 crashes in total in the beta cycle, it looks unlikely to be a problem for the release and we have only 2 betas left, let's have it ride the 84 trains.

Attachment #9184914 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.