227680 - OS takes >30sec to sleep if Camino is running

Status: VERIFIED FIXED

(Camino Graveyard :: General, defect)

Description

[syd]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.6b) Gecko/20031206 Camino/0.7+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.6b) Gecko/20031206 Camino/0.7+

Panther 10.3.1 takes a long time to go to sleep if Camino is running. Quitting
Camino will make Panther sleep immediately.

Reproducible: Always

Steps to Reproduce:
1. Start Camino
2. Select sleep from apple menu

Comment 1

[Jasper]

WOW I can confirm this using the 2003120512 NB. Thanks for finding this issue, I
was wondering what caused Panther to take so long to go to sleep.

Comment 2

[Derek Schrock]

WFM
10.3.1 2003120703 Blue and White/G3

Sleep takes less then 2 seconds

Comment 3

[Noemi]

Actually, I have the same problem in Jaguar 10.2.8 - didn't realize that was the
issue until you brought it up.  hm.

Comment 4

[Jasper]

Perhaps it has to do with the boommarks check (broken bookmarks) that is doen
every 30 seconds. I will notify David and see if he knows something.

Comment 5

[syd]

For memory, I think it crept in around the 0.7+ builds. I can't recall it having
the impact on sleep prior to Panther being released.

Comment 6

[Jasper]

Re: Derek, WFM 20031208 NB.

Comment 7

[Ludovic Hirlimann [:Usul]]

This seems fixed in newer builds. Marking WORKSFORM.

Comment 8

[syd]

Agree. 2003120803 build doesn't have the delay.

Comment 9

[syd]

I've re-encountered the bug, and will post instructions on how to reproduce
shortly. You need to browse to specific web sites before the sleep delay is
introduced. Simply starting Camino and then putting the Mac to sleep may not
exhibit the bug

Comment 10

[bugzilla]

Bug is still there in most recent nightly (Dec. 11)

Steps to reproduce:

1) Open Camino
2) Open a website (example http://www.heise.de)
3) This is the crucial step you've all been missing - at least judging from my
tests: Close window using Cmd-W or the red button 
4) Try going to sleep mode

Sleep problem will remain until browser is restarted.

Comment 11

[syd]

I'll add a few more web sites where it causes this Camino bug as well later.

Comment 12

[Jasper]

Sorry to say so but I can cofrim th issues mentioned in Comment #10. Thi still
doesn't seem to be fixed.

Comment 13

[Ludovic Hirlimann [:Usul]]

setting new.

Comment 14

[Mike Pinkerton (not reading bugmail)]

can someone make a reduced testcase?

Comment 15

[Jasper]

Reduced tescase:

1) launch Camino browse some (doesn't matter which one) wbeiste using the first
window camino made.
2) close the window (and if you wish open a new one)

Result: your mac won't go to sleep for at least another 30 seconds.

Comment 16

[Josh Aas]

I can reproduce this using Jasper's steps with 2003121803 on Mac OS X 10.3.2 (PB
G4 667). Later I'll try to do some task profiling during the time it takes to go
to sleep.

Comment 17

[Martin Girschick]

Has someone checked, whether the mDNSResponder or Rendez-Vous is involved? When
I look at the information in the console before sleep it seems that
mDNSResponder is slowing down the sleep process, although I'm not sure...

Comment 18

[Mike Pinkerton (not reading bugmail)]

smfr's guess (and i agree based on the timing) is that this might be related to
using IOKit to handle gecko's PLEvent handling timers. we probably screwed
something up when the OS asks if we can go to sleep. this code landed around the
same time as this started happening, IIRC.

The code is in mozilla/widget/src/mac/nsToolkitBase.cpp in the
ToolkitSleepWakeCallback()

Comment 19

[Mike Pinkerton (not reading bugmail)]

oh, also....it should be easy to eliminate rendezvous as a culprit by setting a
pref (found in the code) and retesting.

Comment 20

[Josh Aas]

Here are my findings:

- I did a lot of printfs in nsToolkitBase.cpp and everything seems to work fine
so I don't think its a problem with our sleep code so much as the way its handled
- every time a new window is opened, nsToolkitBase gets inited
- the only thing you need to do to cause the 30 second sleep delay is close all
windows

This is going to be long but to sum it up here is a converstion (with fluff
removed) between me and pinkerton:

josh> I think when we close the last window we unregister our stuff from
nsToolkitBase. thats why we're not sleeping. there doesn't seem to be anything
wrong with the actual code in nsToolkitBase.cpp

pinkerton> really?

pinkerton> so it only happens when all windows are closed?

josh> yeah - when there is a window open my thorough printfing is showing that
everything works

<pinkerton> cute

<josh> and every time you open a window nsToolkitBase is inited

<pinkerton> well that would explain a lot then

<josh> but once you close all windows, no sleepy time

<pinkerton> ok, sounds like it's time to make that part a singleton

<pinkerton> by the time we shut down the toolkit, should we have anything that's
calling back for the registrations?

<pinkerton> we shouldn't

<pinkerton> maybe that's part of the prolem where we eat 80% of the cpu with no
windows open

<pinkerton> something is still lingering

<josh> what do you mean "should we have anything that's calling back for the
registrations"?

<josh> the sleep system still would - I don't think it cares whether or not we
have a window open

<pinkerton> but we didn't use to have those routines. you don't NEED them to
sleep unless you're doing something with iokit

<pinkerton> so obvsiouly we're not totally cleaning ourselves up when a window
goes away, right?

<josh> what are we doing? just quartz registration?

<pinkerton> we use IOKit for processing gecko events (PLEvents)

<pinkerton> like timers

<josh>	well, either we're not cleaning ourselves up or we're going too far by
invalidating our callback method when IOKit still expects it to respond

<pinkerton> right, but given that a normal app doesn't need to respond, we must
still be giving it a reason why it thinks it needs to keep asking us

Comment 21

[Jasper]

Just for the record so we don't forget.

With the 20031024 NB this bug isn't reproducable.
With the 20031030 NB this bug isn't reproducable.

This is the same time frame in which Mike temporarely changed from buidling
camino on 10.2 to 10.3 and back. (we think)

With the 20031103 NB and higher this bug is reproducable.

Comment 22

[Mike Pinkerton (not reading bugmail)]

i tried to test this theory about all windows being closed and i put my tibook
to sleep with 1 window open and it still took 30 seconds.

josh: are you sure about that assertion?

Comment 23

[Jasper]

mike: after closing the first window camino creates the issue appears.

Comment 24

[Josh Aas]

Its quite possible, as it seems, that there are other ways to trigger the bug. The only way I could 
consistantly do it was by closing all windows. Opening more and then closing them did not trigger 
the bug for me, but it seems that both of our accounts are in line with what Jasper says about 
closing the original window.

It makes sense that only our first window would be an issue - every other new window is just "re-
registering" (i.e. not registering) with IOKit for sleep notifications. So if the original one isn't 
unregistering correctly its not cleaning up, that is where we would see the problem.

Comment 25

[S Woodside]

What do we do with iokit and where is that code?

Comment 26

[Josh Aas]

The code in question is in:

widget/src/cocoa/nsToolkitBase.cpp

Large portions of it are copied verbatim, with comments, from Apple's example
code. We use it for things related to plevents. If you make changes to that file
you have to 'make' in that directory, then in 'embedding/config' and then again
in the camino dir in order to the changes to propagate up. Problem is we're not
responding to the iokit in certain situations when it asks us if we want to
sleep. When we don't respond, iokit gives us 30 seconds to
respond/deal-with-our-issues then goes to sleep. We don't need to be registered
as an app the iokit needs to ask to sleep all of the time, and we keep getting
asked in situations where we're not preparted to respond.

Comment 27

[Martin Girschick]

I guess you've already seen this, but surely bug #197863 is involved. In the
timeframe mentioned Simon Fraser moved some files around and created
nsToolkitBase.cpp and implemented those sleep routines to fix another bug.

Does anyone know, whether Camino has to reregister for sleep events after a
sleep has happened?

Comment 28

[Simon Fraser [no longer active]]

Simon: LXR is your friend:
<http://lxr.mozilla.org/seamonkey/search?string=iokit>

Comment 29

[S Woodside]

apple's docu on power management has moved here:
http://developer.apple.com/documentation/DeviceDrivers/Conceptual/IOKitFundamentals/PowerMgmt/chapter_10_section_1.html#//apple_ref/doc/uid/TP0000020

Comment 30

[S Woodside]

This is easy to reproduce, add printfs to nsToolkitBase.cpp in
ToolkitSleepWakeCallback. I launch camino, and immediately put the system to
sleep. It sleeps quickly. upon wakeup I have the the following in my run log:

called ToolkitSleepWakeCallback
message was kIOMessageSystemWillSleep
2004-01-17 21:26:59.831 Camino[2962] Updating proxies
2004-01-17 21:27:04.432 Camino[2962] Updating proxies

then my mouse freezes / disappears for about 1-2 seconds and then I get this
message:

called ToolkitSleepWakeCallback

and that's it. So there is an *additional* problem here potentially. But to
consider the problem of delayed sleep first.

Now close the window and sleep again. Observe that it takes 30 seconds to sleep,
and there are no printfs this time.

So, the problem is that the ToolkitSleepWakeCallback is never being called at all.

I also wondering if bug #197863 could have been fixed without involving IOKit
power management functions. It seems that this is a source of potential
complexity and bugs.

Comment 31

[S Woodside]

Attachment: fix

It's a three-line fix for the problem. I added
IODeregisterForSystemPower(&mPowerNotifier); which is necessary to preven IOKit
from attempting to contact the app for power management status. Also I fixed a
typo.

Comment 32

[S Woodside]

even though the patch is to mozilla/widget/src/mac, you have to go to
widget/src/cocoa to make. there's a symlink for nsToolkitBase.cpp there. so
basucally apply the patch, then
cd mozilla/widget/src/cocoa
make
pushd ../../../embedding/config ; make ; popd
then make camino

Comment 33

[Josh Aas]

Comment on attachment 139313 [details] [diff] [review]
fix

looks good, compiles and seems to fix the problem

Comment 34

[Simon Fraser [no longer active]]

Comment on attachment 139313 [details] [diff] [review]
fix

>+  CFRunLoopSourceRef   mSleepWakeNotificationRLS;
>+  io_object_t       mPowerNotifier;

Fix the alignment, and sr=sfraser (clearing bogus self-review request).

Comment 35

[S Woodside]

it's aligned with a whole bunch of other vars further up

Comment 36

[Mike Pinkerton (not reading bugmail)]

landed on trunk

Comment 37

[Chris Petersen]

Using the 2004012303 NB on both a iBook and PowerBook, I can't reproduce the
issue described. Any else see this problem occuring with today's build ?