Printing crashes application in BPrivate::PrintServerMessenger::RejectUserInput()

[diver]

hrev49491. Trying to print or doing page setup from any app causes a crash: for example, Showimage: File->Print... crash.

0x79771880	0xe947a3	BPrivate::PrintServerMessenger::RejectUserInput() + 0x87 
[...]
0x797718b0	0xe94696	__Q28BPrivate20PrintServerMessengerUlP8BMessage + 0x42 
0x79771900	0xe927df	BPrintJob::ConfigJob() + 0x2b 
0x79771ad0	0x120f7cc	ShowImageWindow::_PrepareForPrint() + 0x48 
0x79771bc0	0x120d81b	ShowImageWindow::MessageReceived(BMessage*) + 0x123f 
0x79771bf0	0xe07a99	BLooper::DispatchMessage(BMessage*, BHandler*) + 0x59 
0x79771dd0	0xee7be0	BWindow::DispatchMessage(BMessage*, BHandler*) + 0x176c 
0x79771e30	0xeebbd2	BWindow::task_looper() + 0x28e 
0x79771e60	0xe08f41	BLooper::_task0_(void*) + 0x3d 
0x79771e88	0x2700c7d	thread_entry + 0x21 
00000000	0x609a0250	commpage_thread_exit + 0

[jackburton]

StyledEdit crashes with the same stack trace

[jackburton]

Could be a regression caused by the launch daemon ?

[diver]

Maybe the crash in ​PrintServerMessenger::RejectUserInput() is related to hrev49481?

[jackburton]

Replying to diver:

Maybe the crash in ​PrintServerMessenger::RejectUserInput() is related to hrev49481?

Yeah, that seems a better candidate than the introduction of the launch_daemon

[jackburton]

From a cursory look, one problem seems to be that the window default button is set in BAlert::Go(), while it was set in the constructor before. Since this code dereference DefaultButton() without checking for NULL, this might very well be the cause of the crash.

[jackburton]

Adding the following code:

if (CountButtons() > 0)
		SetDefaultButton(ButtonAt(CountButtons() - 1));

to the end of BAlert::_Init() fixes the crash. Although now we have a nice BAlert shown on the screen. I admit the code in there is sort of a hack, since it creates a BAlert and moves it out of the screen to hide it and block user input.

[jackburton]

Diver, thanks for the hint. It's fixed in hrev49596, although :

1. The code in there doesn't look nice.
2. The changes in hrev49481 could still cause other problems, since it's an API behaviour change.

[jessicah]

With the changes to BAlert, wouldn't it be simpler (and less API breakage) to move the call to SetDefaultButton(ButtonAt(CountButtons() - 1)) inside BAlert::_Prepare() into the new BAlert::AddButton() function? E.g.

void
BAlert::AddButton(const char* label, char key)
{
    ...
    SetDefaultButton(button);
    fButtonLayout->AddView(button);
}

Given that the API documentation states that the last button is, by convention, the default button, and this always creates a new last button, it will automatically set up the default button correctly, as AddButton() is called from _Init() and the existing non-default constructors.

This would remove the hack introduced in hrev49596 for the default button, and avoid possible crashes in other uses of the originally documented BAlert API.