Screen Saver: fix race condition

Start the screensaver in the window thread instead of the runner
thread so that there is no lock contention for the window lock in
the runner thread when the saver starts.

The view that gets drawn into is assumed to have been prepared before
being passed to the runner thread, and this assumption has been made
true for the screensaver preview and screen_blanker apps.

Eliminate fHasStarted and the corresponding HasStarted() method in
ScreenSaverRunner as they are no longer needed.

Drawing still happens in the runner thread, and still needs to lock
the window thread potentially causing contention, yet, there
is a timeout here so the contention won't freeze the screensaver window,
only delay drawing the screensaver.

Drawing could be moved to the window thread via message passing to avoid
lock contention with the window but this would defeat a big part of the
purpose of having a separate rendering thread.

This fixes #10125 and #4260
This commit is contained in:
John Scipione
2013-11-14 23:30:26 -05:00
parent 60c0a74844
commit d34a680c04
8 changed files with 117 additions and 124 deletions
@@ -12,24 +12,19 @@
#define SCREEN_SAVER_RUNNER_H
#include <SupportDefs.h>
#include <DirectWindow.h>
#include <ScreenSaver.h>
#include <View.h>
class BScreenSaver;
class BView;
class ScreenSaverSettings;
#include "ScreenSaverSettings.h"
class ScreenSaverRunner {
public:
ScreenSaverRunner(BWindow* window, BView* view,
bool preview,
ScreenSaverRunner(BView* view,
ScreenSaverSettings& settings);
~ScreenSaverRunner();
BScreenSaver* ScreenSaver() const;
bool HasStarted() const;
BScreenSaver* ScreenSaver() const { return fSaver; };
status_t Run();
void Quit();
@@ -43,13 +38,11 @@ private:
static status_t _ThreadFunc(void* data);
status_t _Run();
BScreenSaver* fSaver;
BWindow* fWindow;
BDirectWindow* fDirectWindow;
BView* fView;
bool fIsDirectDraw;
ScreenSaverSettings& fSettings;
bool fPreview;
bool fHasStarted;
BScreenSaver* fSaver;
image_id fAddonImage;
thread_id fThread;
+16 -13
View File
@@ -40,8 +40,7 @@ ScreenBlanker::ScreenBlanker()
:
BApplication(SCREEN_BLANKER_SIG),
fWindow(NULL),
fSaver(NULL),
fRunner(NULL),
fSaverRunner(NULL),
fPasswordWindow(NULL),
fResumeRunner(NULL),
fStandByScreenRunner(NULL),
@@ -70,15 +69,18 @@ ScreenBlanker::ReadyToRun()
BScreen screen(B_MAIN_SCREEN_ID);
fWindow = new ScreenSaverWindow(screen.Frame());
fPasswordWindow = new PasswordWindow();
fRunner = new ScreenSaverRunner(fWindow, fWindow->ChildAt(0), false, fSettings);
fSaver = fRunner->ScreenSaver();
if (fSaver) {
fWindow->SetSaver(fSaver);
fRunner->Run();
} else {
BView* view = fWindow->ChildAt(0);
fSaverRunner = new ScreenSaverRunner(view, fSettings);
fWindow->SetSaverRunner(fSaverRunner);
BScreenSaver* saver = fSaverRunner->ScreenSaver();
if (saver != NULL && saver->StartSaver(view, false) == B_OK)
fSaverRunner->Run();
else {
fprintf(stderr, "could not load the screensaver addon\n");
fWindow->ChildAt(0)->SetViewColor(0, 0, 0);
view->SetViewColor(0, 0, 0);
// needed for Blackness saver
}
fWindow->SetFullScreen(true);
@@ -110,7 +112,7 @@ ScreenBlanker::_SetDPMSMode(uint32 mode)
screen.SetDPMS(mode);
if (fWindow->Lock()) {
fRunner->Suspend();
fSaverRunner->Suspend();
fWindow->Unlock();
}
}
@@ -122,7 +124,7 @@ ScreenBlanker::_ShowPasswordWindow()
_TurnOnScreen();
if (fWindow->Lock()) {
fRunner->Suspend();
fSaverRunner->Suspend();
fWindow->Sync();
// TODO: is that needed?
@@ -239,7 +241,7 @@ ScreenBlanker::MessageReceived(BMessage* message)
fPasswordWindow->SetPassword("");
fPasswordWindow->Hide();
fRunner->Resume();
fSaverRunner->Resume();
fWindow->Unlock();
}
@@ -301,7 +303,8 @@ ScreenBlanker::_Shutdown()
fWindow->Quit();
}
delete fRunner;
delete fSaverRunner;
fSaverRunner = NULL;
}
+27 -27
View File
@@ -24,37 +24,37 @@ const static uint32 kMsgResumeSaver = 'RSSV';
class ScreenBlanker : public BApplication {
public:
ScreenBlanker();
~ScreenBlanker();
public:
ScreenBlanker();
~ScreenBlanker();
virtual void ReadyToRun();
virtual void ReadyToRun();
virtual bool QuitRequested();
virtual void MessageReceived(BMessage* message);
virtual bool QuitRequested();
virtual void MessageReceived(BMessage* message);
private:
bool _LoadAddOn();
void _ShowPasswordWindow();
void _QueueResumeScreenSaver();
void _TurnOnScreen();
void _SetDPMSMode(uint32 mode);
void _QueueTurnOffScreen();
void _Shutdown();
ScreenSaverSettings fSettings;
ScreenSaverWindow *fWindow;
BScreenSaver *fSaver;
ScreenSaverRunner *fRunner;
PasswordWindow *fPasswordWindow;
bigtime_t fBlankTime;
BMessageRunner* fResumeRunner;
BMessageRunner* fStandByScreenRunner;
BMessageRunner* fSuspendScreenRunner;
BMessageRunner* fTurnOffScreenRunner;
bool IsPasswordWindowShown() const;
private:
bool _LoadAddOn();
void _ShowPasswordWindow();
void _QueueResumeScreenSaver();
void _TurnOnScreen();
void _SetDPMSMode(uint32 mode);
void _QueueTurnOffScreen();
void _Shutdown();
ScreenSaverSettings fSettings;
ScreenSaverWindow* fWindow;
ScreenSaverRunner* fSaverRunner;
PasswordWindow* fPasswordWindow;
bigtime_t fBlankTime;
BMessageRunner* fResumeRunner;
BMessageRunner* fStandByScreenRunner;
BMessageRunner* fSuspendScreenRunner;
BMessageRunner* fTurnOffScreenRunner;
};
#endif // SCREEN_SAVER_APP_H
+23 -15
View File
@@ -71,11 +71,7 @@ ScreenSaverFilter::Filter(BMessage* message, BHandler** target)
}
void
ScreenSaverFilter::SetEnabled(bool enabled)
{
fEnabled = enabled;
}
// #pragma mark - ScreenSaverWindow
/*!
@@ -88,7 +84,9 @@ ScreenSaverWindow::ScreenSaverWindow(BRect frame)
B_NO_BORDER_WINDOW_LOOK, kWindowScreenFeel,
B_NOT_RESIZABLE | B_NOT_MOVABLE | B_NOT_MINIMIZABLE
| B_NOT_ZOOMABLE | B_NOT_CLOSABLE, B_ALL_WORKSPACES),
fSaver(NULL)
fTopView(NULL),
fSaverRunner(NULL),
fFilter(NULL)
{
frame.OffsetTo(0, 0);
fTopView = new BView(frame, "ScreenSaver View", B_FOLLOW_ALL, B_WILL_DRAW);
@@ -99,9 +97,10 @@ ScreenSaverWindow::ScreenSaverWindow(BRect frame)
AddChild(fTopView);
// Ensure that this view receives keyboard input
// Ensure that this view receives keyboard and mouse input
fTopView->MakeFocus(true);
fTopView->SetEventMask(B_KEYBOARD_EVENTS, 0);
fTopView->SetEventMask(B_KEYBOARD_EVENTS | B_POINTER_EVENTS,
B_NO_POINTER_HISTORY);
}
@@ -111,13 +110,6 @@ ScreenSaverWindow::~ScreenSaverWindow()
}
void
ScreenSaverWindow::SetSaver(BScreenSaver *saver)
{
fSaver = saver;
}
void
ScreenSaverWindow::MessageReceived(BMessage* message)
{
@@ -148,3 +140,19 @@ ScreenSaverWindow::DirectConnected(direct_buffer_info* info)
saver->DirectConnected(info);
}
void
ScreenSaverWindow::SetSaverRunner(ScreenSaverRunner* runner)
{
fSaverRunner = runner;
}
BScreenSaver*
ScreenSaverWindow::_ScreenSaver()
{
if (fSaverRunner != NULL)
return fSaverRunner->ScreenSaver();
return NULL;
}
+9 -5
View File
@@ -11,6 +11,8 @@
#include <DirectWindow.h>
#include <MessageFilter.h>
#include "ScreenSaverRunner.h"
const static uint32 kMsgEnableFilter = 'eflt';
@@ -37,16 +39,18 @@ public:
ScreenSaverWindow(BRect frame);
~ScreenSaverWindow();
void SetSaver(BScreenSaver *saver);
virtual void MessageReceived(BMessage* message);
virtual bool QuitRequested();
virtual void DirectConnected(direct_buffer_info* info);
void SetSaverRunner(ScreenSaverRunner* runner);
BScreenSaver* _ScreenSaver();
private:
BView *fTopView;
BScreenSaver *fSaver;
ScreenSaverFilter *fFilter;
private:
BView* fTopView;
ScreenSaverRunner* fSaverRunner;
ScreenSaverFilter* fFilter;
};
#endif // SCREEN_SAVER_WINDOW_H
+21 -38
View File
@@ -11,29 +11,27 @@
#include "ScreenSaverRunner.h"
#include "ScreenSaverSettings.h"
#include <FindDirectory.h>
#include <Screen.h>
#include <ScreenSaver.h>
#include <View.h>
#include <stdio.h>
#include <DirectWindow.h>
#include <FindDirectory.h>
#include <Message.h>
#include <Window.h>
ScreenSaverRunner::ScreenSaverRunner(BWindow* window, BView* view,
bool preview, ScreenSaverSettings& settings)
ScreenSaverRunner::ScreenSaverRunner(BView* view,
ScreenSaverSettings& settings)
:
fSaver(NULL),
fWindow(window),
fView(view),
fIsDirectDraw(view != NULL
&& dynamic_cast<BDirectWindow*>(view->Window()) != NULL),
fSettings(settings),
fPreview(preview),
fSaver(NULL),
fAddonImage(-1),
fThread(-1),
fQuitting(false)
{
fDirectWindow = dynamic_cast<BDirectWindow *>(fWindow);
_LoadAddOn();
}
@@ -47,20 +45,6 @@ ScreenSaverRunner::~ScreenSaverRunner()
}
BScreenSaver*
ScreenSaverRunner::ScreenSaver() const
{
return fSaver;
}
bool
ScreenSaverRunner::HasStarted() const
{
return fHasStarted;
}
status_t
ScreenSaverRunner::Run()
{
@@ -184,13 +168,12 @@ ScreenSaverRunner::_Run()
{
static const uint32 kInitialTickRate = 50000;
if (fWindow->Lock()) {
fView->SetViewColor(0, 0, 0);
fView->SetLowColor(0, 0, 0);
if (fView == NULL || fView->Window() == NULL) {
// view is NULL or not connected to app server, bail out
if (fSaver != NULL)
fHasStarted = fSaver->StartSaver(fView, fPreview) == B_OK;
fSaver->StopSaver();
fWindow->Unlock();
return B_BAD_VALUE;
}
// TODO: This code is getting awfully complicated and should
@@ -199,7 +182,7 @@ ScreenSaverRunner::_Run()
int32 snoozeCount = 0;
int32 frame = 0;
bigtime_t lastTickTime = 0;
bigtime_t tick = fSaver ? fSaver->TickSize() : tickBase;
bigtime_t tick = fSaver != NULL ? fSaver->TickSize() : tickBase;
while (!fQuitting) {
// break the idle time up into ticks so that we can evaluate
@@ -214,7 +197,7 @@ ScreenSaverRunner::_Run()
// re-evaluate the tick time after each successful wakeup
// screensavers can adjust it on the fly, and we must be
// prepared to accomodate that
tick = fSaver ? fSaver->TickSize() : tickBase;
tick = fSaver != NULL ? fSaver->TickSize() : tickBase;
if (tick < tickBase) {
if (tick < 0)
@@ -229,22 +212,22 @@ ScreenSaverRunner::_Run()
if (snoozeCount) {
// if we are sleeping, do nothing
snoozeCount--;
} else if (fSaver != NULL && fHasStarted) {
} else if (fSaver != NULL) {
if (fSaver->LoopOnCount() && frame >= fSaver->LoopOnCount()) {
// Time to nap
frame = 0;
snoozeCount = fSaver->LoopOffCount();
} else if (fWindow->LockWithTimeout(5000LL) == B_OK) {
} else if (fView->Window()->LockWithTimeout(5000LL) == B_OK) {
if (!fQuitting) {
// NOTE: R5 really calls DirectDraw()
// NOTE: BeOS R5 really calls DirectDraw()
// and then Draw() for the same frame
if (fDirectWindow)
if (fIsDirectDraw)
fSaver->DirectDraw(frame);
fSaver->Draw(fView, frame);
fView->Sync();
frame++;
}
fWindow->Unlock();
fView->Window()->Unlock();
}
} else
snoozeCount = 1000;
@@ -19,11 +19,13 @@
#include <Box.h>
#include <Button.h>
#include <Catalog.h>
#include <CheckBox.h>
#include <ControlLook.h>
#include <Directory.h>
#include <DurationFormat.h>
#include <Entry.h>
#include <File.h>
#include <FilePanel.h>
#include <FindDirectory.h>
#include <Font.h>
#include <Layout.h>
@@ -31,6 +33,7 @@
#include <ListItem.h>
#include <ListView.h>
#include <Path.h>
#include <Rect.h>
#include <Roster.h>
#include <Screen.h>
#include <ScreenSaver.h>
@@ -807,8 +810,7 @@ ModulesView::_OpenSaver()
BView* view = fPreviewView->AddPreview();
fCurrentName = fSettings.ModuleName();
fSaverRunner = new ScreenSaverRunner(Window(), view, true, fSettings);
BScreenSaver* saver = _ScreenSaver();
fSaverRunner = new ScreenSaverRunner(view, fSettings);
#ifdef __HAIKU__
BRect rect = fSettingsBox->InnerFrame().InsetByCopy(4, 4);
@@ -821,8 +823,12 @@ ModulesView::_OpenSaver()
fSettingsView->SetViewColor(ui_color(B_PANEL_BACKGROUND_COLOR));
fSettingsBox->AddChild(fSettingsView);
if (saver != NULL && fSaverRunner->Run() == B_OK)
BScreenSaver* saver = _ScreenSaver();
if (saver != NULL && fSettingsView != NULL) {
saver->StartConfig(fSettingsView);
if (saver->StartSaver(view, false) == B_OK)
fSaverRunner->Run();
}
if (fSettingsView->ChildAt(0) == NULL) {
// There are no settings at all, we add the module name here to
@@ -12,18 +12,14 @@
#define SCREEN_SAVER_WINDOW_H
#include <Window.h>
#include "PasswordWindow.h"
#include <Box.h>
#include <CheckBox.h>
#include <FilePanel.h>
#include <Slider.h>
#include <ListView.h>
#include "ScreenSaverSettings.h"
class BButton;
class BMessage;
class BRect;
class BTabView;
class FadeView;