Resolved race condition in methode Go of dialog windows.

git-svn-id: file:///srv/svn/repos/haiku/trunk/current@10276 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/print/libprint/DialogWindow.h

@@ -0,0 +1,42 @@
+/*
+ * DialogWindow.h
+ * Copyright 2004 Michael Pfeiffer. All Rights Reserved.
+ */
+ 
+#ifndef __DIALOG_WINDOW_H
+#define __DIALOG_WINDOW_H
+
+#include <OS.h>
+#include <Window.h>
+
+class DialogWindow : public BWindow {
+public:
+	DialogWindow(BRect frame,
+			const char *title, 
+			window_type type,
+			uint32 flags,
+			uint32 workspace = B_CURRENT_WORKSPACE);
+	
+	DialogWindow(BRect frame,
+			const char *title, 
+			window_look look,
+			window_feel feel,
+			uint32 flags,
+			uint32 workspace = B_CURRENT_WORKSPACE);
+			
+	status_t Go();
+	
+	void SetResult(status_t result);
+
+	void MessageReceived(BMessage* msg);
+	
+	enum {
+		kGetThreadId = 'dwti' // request thread id from window
+	};
+		
+private:
+	status_t           fPreviousResult; // holds the result as long as fResult == NULL
+	volatile status_t *fResult;
+};
+
+#endif

headers/private/print/libprint/JobSetupDlg.h

@@ -7,16 +7,16 @@
 #define __JOBSETUPDLG_H
 
 #include <View.h>
-#include <Window.h>
+#include "DialogWindow.h"
 
 #include "JobData.h"
 #include "Halftone.h"
+#include "JSDSlider.h"
 
 class BTextControl;
 class BRadioButton;
 class BCheckBox;
 class BPopUpMenu;
-class BSlider;
 class JobData;
 class PrinterData;
 class PrinterCap;
@@ -45,8 +45,8 @@ private:
 	const PrinterCap *fPrinterCap;
 	BPopUpMenu       *fColorType;
 	BPopUpMenu       *fDitherType;
-	BSlider          *fGamma;
+	JSDSlider        *fGamma;
-	BSlider          *fInkDensity;
+	JSDSlider        *fInkDensity;
 	HalftoneView     *fHalftone;
 	BRadioButton     *fAll;
 	BCheckBox        *fCollate;
@@ -56,17 +56,13 @@ private:
 	BPopUpMenu       *fNup;
 };
 
-class JobSetupDlg : public BWindow {
+class JobSetupDlg : public DialogWindow {
 public:
 	JobSetupDlg(JobData *job_data, PrinterData *printer_data, const PrinterCap *printer_cap);
 	~JobSetupDlg();
-	virtual bool QuitRequested();
 	virtual	void MessageReceived(BMessage *message);
-	int Go();
 
 private:
-	int  fResult;
-	long fSemaphore;
 	BMessageFilter *fFilter;
 };
 

headers/private/print/libprint/PageSetupDlg.h

@@ -7,7 +7,7 @@
 #define __PAGESETUPDLG_H
 
 #include <View.h>
-#include <Window.h>
+#include "DialogWindow.h"
 
 class BRadioButton;
 class BPopUpMenu;
@@ -31,17 +31,12 @@ private:
 	BPopUpMenu       *fResolution;
 };
 
-class PageSetupDlg : public BWindow {
+class PageSetupDlg : public DialogWindow {
 public:
 	PageSetupDlg(JobData *job_data, PrinterData *printer_data, const PrinterCap *printer_cap);
-	~PageSetupDlg();
-	virtual bool QuitRequested();
 	virtual void MessageReceived(BMessage *message);
-	int Go();
 
 private:
-	int  fResult;
-	long fSemaphore;
 	BMessageFilter *fFilter;
 };
 

src/add-ons/print/drivers/shared/libprint/DialogWindow.cpp

@@ -0,0 +1,88 @@
+/*
+ * DialogWindow.h
+ * Copyright 2004 Michael Pfeiffer. All Rights Reserved.
+ */
+ 
+#include "DialogWindow.h"
+
+#include <Messenger.h>
+
+DialogWindow::DialogWindow(BRect frame,
+			const char *title, 
+			window_type type,
+			uint32 flags,
+			uint32 workspace)
+	: BWindow(frame, title, type, flags, workspace)
+	, fPreviousResult(B_OK)
+	, fResult(NULL)
+{
+	// nothing to do
+}
+
+DialogWindow::DialogWindow(BRect frame,
+			const char *title, 
+			window_look look,
+			window_feel feel,
+			uint32 flags,
+			uint32 workspace)
+	: BWindow(frame, title, look, feel, flags, workspace)
+	, fPreviousResult(B_OK)
+	, fResult(NULL)
+{
+	// nothing to do
+}
+
+void DialogWindow::MessageReceived(BMessage *msg)
+{
+	if (msg->what == kGetThreadId) {
+		BMessage reply;
+		reply.AddInt32("thread_id", Thread());
+		msg->SendReply(&reply);
+		return;
+	}
+	BWindow::MessageReceived(msg);
+}
+			
+status_t DialogWindow::Go()
+{
+	BMessenger messenger(this, this);
+	// store result in local variable and
+	// initialize it with previous result
+	volatile status_t result = fPreviousResult;
+	// new results are stored on the stack
+	fResult = &result;
+	
+	// show the window
+	Show();
+	// at this point we must not access member variables,
+	// because this object (the window) could already be deleted.
+
+	// get thread id of window thread
+	BMessage reply;
+	if (messenger.SendMessage(kGetThreadId, &reply) != B_OK) {
+		return B_ERROR;
+	}
+	thread_id windowThread;
+	if (reply.FindInt32("thread_id", &windowThread) != B_OK) {
+		return B_ERROR;
+	}
+	
+	// wait for window thread to die
+	// The window thread will crash if the image holding the
+	// code used by the window thread is unloaded while the thread is 
+	// still running!!!
+	status_t status = B_ERROR;
+	wait_for_thread(windowThread, &status);
+	
+	return result;
+}
+	
+void DialogWindow::SetResult(status_t result)
+{
+	if (fResult != NULL) {
+		*fResult = result;
+	} else {
+		fPreviousResult = result;
+	}
+}
+

src/add-ons/print/drivers/shared/libprint/JobSetupDlg.cpp

@@ -35,6 +35,7 @@
 #include "HalftoneView.h"
 #include "JobSetupDlg.h"
 #include "JobData.h"
+#include "JSDSlider.h"
 #include "PrinterData.h"
 #include "PrinterCap.h"
 #include "DbgMsg.h"
@@ -439,7 +440,8 @@ void JobSetupView::AttachedToWindow()
 	fHalftone->preview(fJobData->getGamma(), fJobData->getInkDensity(), fJobData->getDitherType(), fJobData->getColor() == JobData::kColor);
 	
 	/* gamma */
-	fGamma = new BSlider(gamma_rect, "", "Gamma", new BMessage(kMsgQuality), -300, 300);
+	fGamma = new JSDSlider(gamma_rect, "gamma", "Gamma", new BMessage(kMsgQuality), -300, 300, B_BLOCK_THUMB);
+	
 	fGamma->SetLimitLabels("Brighter", "Darker");
 	fGamma->SetValue(100 * log(fJobData->getGamma()) / log(2.0));
 	fGamma->SetHashMarks(B_HASH_MARKS_BOTH);
@@ -449,7 +451,8 @@ void JobSetupView::AttachedToWindow()
 	fGamma->SetTarget(this);
 
 	/* ink density */
-	fInkDensity = new BSlider(ink_density_rect, "", "Ink Density", new BMessage(kMsgQuality), 0, 127);
+	fInkDensity = new JSDSlider(ink_density_rect, "inkDensity", "Ink Density", new BMessage(kMsgQuality), 0, 127, B_BLOCK_THUMB);
+	
 	fInkDensity->SetLimitLabels("Max", "Min");
 	fInkDensity->SetValue((int32)fJobData->getInkDensity());
 	fInkDensity->SetHashMarks(B_HASH_MARKS_TOP);
@@ -771,70 +774,44 @@ filter_result PrintKeyFilter(BMessage *msg, BHandler **target, BMessageFilter *f
 //====================================================================
 
 JobSetupDlg::JobSetupDlg(JobData *job_data, PrinterData *printer_data, const PrinterCap *printer_cap)
-	: BWindow(BRect(100, 100, 100 + PRINT_WIDTH, 100 + PRINT_HEIGHT),
+	: DialogWindow(BRect(100, 100, 100 + PRINT_WIDTH, 100 + PRINT_HEIGHT),
 		"PrintJob Setup", B_TITLED_WINDOW_LOOK, B_MODAL_APP_WINDOW_FEEL,
 		B_NOT_RESIZABLE | B_NOT_MINIMIZABLE | B_NOT_ZOOMABLE | B_ASYNCHRONOUS_CONTROLS)
 {
-	fResult = 0;
 /*
 	ostringstream oss;
 	oss << printer_data->get_printer_name() << " Print";
 	SetTitle(oss.str().c_str());
 */
-	Lock();
+	SetResult(B_ERROR);
+	
 	JobSetupView *view = new JobSetupView(Bounds(), job_data, printer_data, printer_cap);
 	AddChild(view);
 	fFilter = new BMessageFilter(B_ANY_DELIVERY, B_ANY_SOURCE, B_KEY_DOWN, &PrintKeyFilter);
 	AddCommonFilter(fFilter);
-	Unlock();
-
-	fSemaphore = create_sem(0, "JobSetupSem");
 }
 
 JobSetupDlg::~JobSetupDlg()
 {
-	Lock();
 	RemoveCommonFilter(fFilter);
-	Unlock();
 	delete fFilter;
 }
 
-bool JobSetupDlg::QuitRequested()
-{
-	fResult = B_ERROR;
-	release_sem(fSemaphore);
-	return true;
-}
-
 void JobSetupDlg::MessageReceived(BMessage *msg)
 {
 	switch (msg->what) {
 	case kMsgOK:
-		Lock();
 		((JobSetupView *)ChildAt(0))->UpdateJobData();
-		Unlock();
+		SetResult(B_NO_ERROR);
-		fResult = B_NO_ERROR;
+		PostMessage(B_QUIT_REQUESTED);
-		release_sem(fSemaphore);
 		break;
 
 	case kMsgCancel:
-		fResult = B_ERROR;
+		PostMessage(B_QUIT_REQUESTED);
-		release_sem(fSemaphore);
 		break;
 	
 	default:
-		BWindow::MessageReceived(msg);
+		DialogWindow::MessageReceived(msg);
 		break;
 	}
 }
-
-int JobSetupDlg::Go()
-{
-	Show();
-	acquire_sem(fSemaphore);
-	delete_sem(fSemaphore);
-	int value = fResult;
-	Lock();
-	Quit();
-	return value;
-}

src/add-ons/print/drivers/shared/libprint/PageSetupDlg.cpp

@@ -270,34 +270,20 @@ bool PageSetupView::UpdateJobData()
 //====================================================================
 
 PageSetupDlg::PageSetupDlg(JobData *job_data, PrinterData *printer_data, const PrinterCap *printer_cap)
-	: BWindow(BRect(100, 100, 100 + PAGESETUP_WIDTH, 100 + PAGESETUP_HEIGHT),
+	: DialogWindow(BRect(100, 100, 100 + PAGESETUP_WIDTH, 100 + PAGESETUP_HEIGHT),
 		"Page Setup", B_TITLED_WINDOW_LOOK, B_MODAL_APP_WINDOW_FEEL,
 		B_NOT_RESIZABLE | B_NOT_MINIMIZABLE | B_NOT_ZOOMABLE)
 {
-	fResult = 0;
 /*
 	ostringstream oss;
 	oss << printer_data->get_printer_name() << " Setup";
 	SetTitle(title.str().c_str());
 */
 
-	Lock();
 	PageSetupView *view = new PageSetupView(Bounds(), job_data, printer_data, printer_cap);
 	AddChild(view);
-	Unlock();
 	
-	fSemaphore = create_sem(0, "PageSetupSem");
+	SetResult(B_ERROR);
-}
-
-PageSetupDlg::~PageSetupDlg()
-{
-}
-
-bool PageSetupDlg::QuitRequested()
-{
-	fResult = B_ERROR;
-	release_sem(fSemaphore);
-	return true;
 }
 
 void PageSetupDlg::MessageReceived(BMessage *msg)
@@ -307,28 +293,16 @@ void PageSetupDlg::MessageReceived(BMessage *msg)
 		Lock();
 		((PageSetupView *)ChildAt(0))->UpdateJobData();
 		Unlock();
-		fResult = B_NO_ERROR;
+		SetResult(B_NO_ERROR);
-		release_sem(fSemaphore);
+		PostMessage(B_QUIT_REQUESTED);
 		break;
 
 	case kMsgCancel:
-		fResult = B_ERROR;
+		PostMessage(B_QUIT_REQUESTED);
-		release_sem(fSemaphore);
 		break;
 
 	default:
-		BWindow::MessageReceived(msg);
+		DialogWindow::MessageReceived(msg);
-		break;
 	}
 }
 
-int PageSetupDlg::Go()
-{
-	Show();
-	acquire_sem(fSemaphore);
-	delete_sem(fSemaphore);
-	int value = fResult;
-	Lock();
-	Quit();
-	return value;
-}