View Issue Details

IDProjectCategoryView StatusLast Update
0034798LazarusLCLpublic2019-05-23 19:04
ReporterZoë PetersonAssigned ToDmitry Boyarintsev 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product VersionProduct Build 
Target VersionFixed in Version 
Summary0034798: [Patch] Improve "Quit App" and system shutdown/logout
DescriptionLCL/Cocoa's "Quit Application" menu item currently blindly calls "Application.Mainform.Close", which crashes if the mainform isn't assigned. The widgetset also doesn't listen for "Quit" events coming from the system, so if the app is running when the user tries to shutdown or log out, it's killed without running any of the usual OnCloseQuery/OnClose/finalization blocks.

This patch cleans everything up by doing the following:

1) The "Quit Application" menu now calls a new TCocoaWidgetSet.QuitApp method. QuitApp calls a new TCocoaWidgetSet.OnQuitApp event and then calls Application.MainForm.Close or Application.Terminate. This allows third-party apps to trap it and do any app-wide confirmations.

2) Adds an event handler for the "kAEQuitApplication" Apple Event, which the system sends when the user has requested a shutdown or logout, or if the user or another app uses Apple Script to tell the app to close. The event handler likewise goes through TCocoaWidgetSet.QuitApp, so everything is centralized.

3) The kAEQuitApplication event handler also detects if the quit request was system-wide and calls TApplication's OnQueryEndSession/OnEndSession events if it is. Neither Carbon nor Cocoa did this previously, but the Win32 and Qt widgetset do.
Steps To Reproduce**quitapp-Shutdown.zip** is a sample project that overrides TForm OnCloseQuery and OnClose, TApplication OnQueryEndSession and OnEndSession, and finalization. With the patch, the event handlers are all called in the same order as Win32 for both closing the application and shutting down, and you can cancel in either OnCloseQuery or OnQueryEndSession.

**quitapp-NoMainForm.zip** is a sample project that just creates a global menu without a corresponding form and waits until the user selects "Quit". I didn't include event handlers since it isn't necessary to show the desired behavior.
Additional Information1) The most "Cocoa" way to handle shutdown is by overriding **applicationShouldTerminate:** in the app delegate. That doesn't work because after it returns the system calls applicationWillTerminate: and then immediately kills the application. We don't get a chance to run any code after Application.Run in the lpr, and none of the finalization sections run. Calling Application.Terminate and returning terminateLater also doesn't work, because it starts a modal message loop, so we never get back to the LCL RunLoop. LCLCOCOA already avoids using [NSApp terminate] for that exact reason.

2) The event handler we are adding needs to be assigned in applicationWillFinishLaunching: to take effect. Calling setEventHandler_... immediately after creating the app delegate doesn't work.

3) OnQuitApp could be exposed at a higher level to avoid needing to interact with TCocoaWidgetSet directly, but since it's Cocoa-specific behavior, it seemed appropriate to put it there. TWin32WidgetSet has an OnAsyncSocketMsg event that's similarly exposed.

4) OnQuitApp wasn't necessary on LCLCarbon because the system-provided "Quit Application" menu item also sent the kAEQuitApplication event, so we could just trap that. To get the same behavior you could move the MainForm.Close call into the event handler and have TCocoaMenuItem_Quit.lclItemSelected create and send a kAEQuitApplication event to the current process, but that seems messier and less pleasant to use.
TagsNo tags attached.
Fixed in Revision61230
LazTarget-
WidgetsetCocoa
Attached Files
  • quitapp.patch (4,330 bytes)
    Index: cocoaint.pas
    ===================================================================
    --- cocoaint.pas	(revision 59976)
    +++ cocoaint.pas	(working copy)
    @@ -58,6 +58,8 @@
         procedure applicationDidBecomeActive(notification: NSNotification);
         procedure applicationDidResignActive(notification: NSNotification);
         procedure applicationDidChangeScreenParameters(notification: NSNotification);
    +    procedure applicationWillFinishLaunching(notification: NSNotification);
    +    procedure handleQuitAppEvent_withReplyEvent(event: NSAppleEventDescriptor; replyEvent: NSAppleEventDescriptor); message 'handleQuitAppEvent:withReplyEvent:';
       end;
     
       { TCocoaApplication }
    @@ -102,6 +104,7 @@
         FNSApp_Delegate: TAppDelegate;
         FCurrentCursor: HCursor;
         FCaptureControl: HWND;
    +    FOnQuitApp: TCloseQueryEvent;
     
       protected
         FStockNullBrush: HBRUSH;
    @@ -179,6 +182,8 @@
         procedure EndModal(awin: NSWindow);
         function isModalSession: Boolean;
     
    +    procedure QuitApp;
    +
         {todo:}
         function  DCGetPixel(CanvasHandle: HDC; X, Y: integer): TGraphicsColor; override;
         procedure DCSetPixel(CanvasHandle: HDC; X, Y: integer; AColor: TGraphicsColor); override;
    @@ -198,6 +203,8 @@
         {$I cocoawinapih.inc}
         // the extra LCL interface methods
         {$I cocoalclintfh.inc}
    +
    +    property OnQuitApp: TCloseQueryEvent read FOnQuitApp write FOnQuitApp;
       end;
       
     var
    @@ -652,6 +659,21 @@
       Result := Assigned(Modals) and (Modals.Count > 0);
     end;
     
    +procedure TCocoaWidgetSet.QuitApp;
    +var
    +  CanClose: Boolean;
    +begin
    +  // Should be used instead of Application.Terminate to allow events to be sent, see bug 32148
    +  CanClose := True;
    +  if Assigned(OnQuitApp) then
    +    OnQuitApp(Self, CanClose);
    +  if CanClose then
    +    if Assigned(Application.MainForm) then
    +      Application.MainForm.Close
    +    else
    +      Application.Terminate;
    +end;
    +
     initialization
     //  {$I Cocoaimages.lrs}
     
    Index: cocoaobject.inc
    ===================================================================
    --- cocoaobject.inc	(revision 59976)
    +++ cocoaobject.inc	(working copy)
    @@ -603,6 +603,43 @@
       Screen.UpdateScreen;
     end;
     
    +procedure TAppDelegate.applicationWillFinishLaunching(notification: NSNotification);
    +begin
    +  NSAppleEventManager.sharedAppleEventManager.setEventHandler_andSelector_forEventClass_andEventID(
    +    Self, ObjCSelector('handleQuitAppEvent:withReplyEvent:'), kCoreEventClass,
    +    kAEQuitApplication);
    +end;
    +
    +procedure TAppDelegate.handleQuitAppEvent_withReplyEvent(event: NSAppleEventDescriptor; replyEvent: NSAppleEventDescriptor);
    +{ Capture "Quit Application" Apple Events, either from system shutdown/logout
    +  or sent by another application.  Don't use [applicationShouldTerminate:]
    +  because that terminates the app immediately after [applicationWillTerminate:]
    +  returns, so there's no chance to run finalization blocks }
    +var
    +  Cancel: Boolean;
    +  Reason: NSAppleEventDescriptor;
    +begin
    +  Cancel := False;
    +  // Check if it's a system-wide event
    +  Reason := event.attributeDescriptorForKeyword(kEventParamReason);
    +  if (Reason <> nil) and
    +     ((Reason.typeCodeValue = kAEQuitAll) or
    +      (reason.typeCodeValue = kAEReallyLogOut) or
    +      (reason.typeCodeValue = kAERestart) or
    +      (reason.typeCodeValue = kAEShutDown)) then
    +  begin
    +    Application.IntfQueryEndSession(Cancel);
    +    if not Cancel then
    +      Application.IntfEndSession;
    +  end;
    +  // Try to quit
    +  if not Cancel then
    +    CocoaWidgetSet.QuitApp;
    +  // Let caller know if the shutdown was cancelled
    +  if (not Application.Terminated) and (replyEvent.descriptorType <> typeNull) then
    +    replyEvent.setParamDescriptor_forKeyword(NSAppleEventDescriptor.descriptorWithInt32(userCanceledErr), keyErrorNumber);
    +end;
    +
     {------------------------------------------------------------------------------
       Method:  TCocoaWidgetSet.RawImage_DescriptionFromCocoaBitmap
     
    Index: cocoawsmenus.pas
    ===================================================================
    --- cocoawsmenus.pas	(revision 59976)
    +++ cocoawsmenus.pas	(working copy)
    @@ -395,8 +395,7 @@
     
     procedure TCocoaMenuItem_Quit.lclItemSelected(sender: id);
     begin
    -  // Should be used instead of Application.Terminate to allow events to be sent, see bug 32148
    -  Application.MainForm.Close;
    +  CocoaWidgetSet.QuitApp;
     end;
     
     { TCocoaWSMenu }
    
    quitapp.patch (4,330 bytes)
  • quitapp-NoMainForm.zip (66,022 bytes)
  • quitapp-Shutdown.zip (67,744 bytes)
  • cocoa-shutdown.patch (2,753 bytes)
    diff --git a/lcl/interfaces/cocoa/cocoaint.pas b/lcl/interfaces/cocoa/cocoaint.pas
    index ac6e600e55..69da9addbf 100644
    --- a/lcl/interfaces/cocoa/cocoaint.pas
    +++ b/lcl/interfaces/cocoa/cocoaint.pas
    @@ -58,6 +58,8 @@ type
         procedure applicationDidBecomeActive(notification: NSNotification);
         procedure applicationDidResignActive(notification: NSNotification);
         procedure applicationDidChangeScreenParameters(notification: NSNotification);
    +    procedure applicationWillFinishLaunching(notification: NSNotification);
    +    procedure handleQuitAppEvent_withReplyEvent(event: NSAppleEventDescriptor; replyEvent: NSAppleEventDescriptor); message 'handleQuitAppEvent:withReplyEvent:';
       end;
     
       { TCocoaApplication }
    diff --git a/lcl/interfaces/cocoa/cocoaobject.inc b/lcl/interfaces/cocoa/cocoaobject.inc
    index 5a17d799d0..2ea07501dc 100644
    --- a/lcl/interfaces/cocoa/cocoaobject.inc
    +++ b/lcl/interfaces/cocoa/cocoaobject.inc
    @@ -592,6 +592,43 @@ begin
       Screen.UpdateScreen;
     end;
     
    +procedure TAppDelegate.applicationWillFinishLaunching(notification: NSNotification);
    +begin
    +  NSAppleEventManager.sharedAppleEventManager.setEventHandler_andSelector_forEventClass_andEventID(
    +    Self, ObjCSelector('handleQuitAppEvent:withReplyEvent:'), kCoreEventClass,
    +    kAEQuitApplication);
    +end;
    +
    +procedure TAppDelegate.handleQuitAppEvent_withReplyEvent(event: NSAppleEventDescriptor; replyEvent: NSAppleEventDescriptor);
    +{ Capture "Quit Application" Apple Events, either from system shutdown/logout
    +  or sent by another application.  Don't use [applicationShouldTerminate:]
    +  because that terminates the app immediately after [applicationWillTerminate:]
    +  returns, so there's no chance to run finalization blocks }
    +var
    +  Cancel: Boolean;
    +  Reason: NSAppleEventDescriptor;
    +begin
    +  Cancel := False;
    +  // Check if it's a system-wide event
    +  Reason := event.attributeDescriptorForKeyword(kEventParamReason);
    +  if (Reason <> nil) and
    +     ((Reason.typeCodeValue = kAEQuitAll) or
    +      (reason.typeCodeValue = kAEReallyLogOut) or
    +      (reason.typeCodeValue = kAERestart) or
    +      (reason.typeCodeValue = kAEShutDown)) then
    +  begin
    +    Application.IntfQueryEndSession(Cancel);
    +    if not Cancel then
    +      Application.IntfEndSession;
    +  end;
    +  // Try to quit
    +  if not Cancel then
    +    Application.MainForm.Close;
    +  // Let caller know if the shutdown was cancelled
    +  if (not Application.Terminated) and (replyEvent.descriptorType <> typeNull) then
    +    replyEvent.setParamDescriptor_forKeyword(NSAppleEventDescriptor.descriptorWithInt32(userCanceledErr), keyErrorNumber);
    +end;
    +
     {------------------------------------------------------------------------------
       Method:  TCocoaWidgetSet.RawImage_DescriptionFromCocoaBitmap
     
    
    cocoa-shutdown.patch (2,753 bytes)

Activities

Zoë Peterson

2019-01-03 00:22

reporter  

quitapp.patch (4,330 bytes)
Index: cocoaint.pas
===================================================================
--- cocoaint.pas	(revision 59976)
+++ cocoaint.pas	(working copy)
@@ -58,6 +58,8 @@
     procedure applicationDidBecomeActive(notification: NSNotification);
     procedure applicationDidResignActive(notification: NSNotification);
     procedure applicationDidChangeScreenParameters(notification: NSNotification);
+    procedure applicationWillFinishLaunching(notification: NSNotification);
+    procedure handleQuitAppEvent_withReplyEvent(event: NSAppleEventDescriptor; replyEvent: NSAppleEventDescriptor); message 'handleQuitAppEvent:withReplyEvent:';
   end;
 
   { TCocoaApplication }
@@ -102,6 +104,7 @@
     FNSApp_Delegate: TAppDelegate;
     FCurrentCursor: HCursor;
     FCaptureControl: HWND;
+    FOnQuitApp: TCloseQueryEvent;
 
   protected
     FStockNullBrush: HBRUSH;
@@ -179,6 +182,8 @@
     procedure EndModal(awin: NSWindow);
     function isModalSession: Boolean;
 
+    procedure QuitApp;
+
     {todo:}
     function  DCGetPixel(CanvasHandle: HDC; X, Y: integer): TGraphicsColor; override;
     procedure DCSetPixel(CanvasHandle: HDC; X, Y: integer; AColor: TGraphicsColor); override;
@@ -198,6 +203,8 @@
     {$I cocoawinapih.inc}
     // the extra LCL interface methods
     {$I cocoalclintfh.inc}
+
+    property OnQuitApp: TCloseQueryEvent read FOnQuitApp write FOnQuitApp;
   end;
   
 var
@@ -652,6 +659,21 @@
   Result := Assigned(Modals) and (Modals.Count > 0);
 end;
 
+procedure TCocoaWidgetSet.QuitApp;
+var
+  CanClose: Boolean;
+begin
+  // Should be used instead of Application.Terminate to allow events to be sent, see bug 32148
+  CanClose := True;
+  if Assigned(OnQuitApp) then
+    OnQuitApp(Self, CanClose);
+  if CanClose then
+    if Assigned(Application.MainForm) then
+      Application.MainForm.Close
+    else
+      Application.Terminate;
+end;
+
 initialization
 //  {$I Cocoaimages.lrs}
 
Index: cocoaobject.inc
===================================================================
--- cocoaobject.inc	(revision 59976)
+++ cocoaobject.inc	(working copy)
@@ -603,6 +603,43 @@
   Screen.UpdateScreen;
 end;
 
+procedure TAppDelegate.applicationWillFinishLaunching(notification: NSNotification);
+begin
+  NSAppleEventManager.sharedAppleEventManager.setEventHandler_andSelector_forEventClass_andEventID(
+    Self, ObjCSelector('handleQuitAppEvent:withReplyEvent:'), kCoreEventClass,
+    kAEQuitApplication);
+end;
+
+procedure TAppDelegate.handleQuitAppEvent_withReplyEvent(event: NSAppleEventDescriptor; replyEvent: NSAppleEventDescriptor);
+{ Capture "Quit Application" Apple Events, either from system shutdown/logout
+  or sent by another application.  Don't use [applicationShouldTerminate:]
+  because that terminates the app immediately after [applicationWillTerminate:]
+  returns, so there's no chance to run finalization blocks }
+var
+  Cancel: Boolean;
+  Reason: NSAppleEventDescriptor;
+begin
+  Cancel := False;
+  // Check if it's a system-wide event
+  Reason := event.attributeDescriptorForKeyword(kEventParamReason);
+  if (Reason <> nil) and
+     ((Reason.typeCodeValue = kAEQuitAll) or
+      (reason.typeCodeValue = kAEReallyLogOut) or
+      (reason.typeCodeValue = kAERestart) or
+      (reason.typeCodeValue = kAEShutDown)) then
+  begin
+    Application.IntfQueryEndSession(Cancel);
+    if not Cancel then
+      Application.IntfEndSession;
+  end;
+  // Try to quit
+  if not Cancel then
+    CocoaWidgetSet.QuitApp;
+  // Let caller know if the shutdown was cancelled
+  if (not Application.Terminated) and (replyEvent.descriptorType <> typeNull) then
+    replyEvent.setParamDescriptor_forKeyword(NSAppleEventDescriptor.descriptorWithInt32(userCanceledErr), keyErrorNumber);
+end;
+
 {------------------------------------------------------------------------------
   Method:  TCocoaWidgetSet.RawImage_DescriptionFromCocoaBitmap
 
Index: cocoawsmenus.pas
===================================================================
--- cocoawsmenus.pas	(revision 59976)
+++ cocoawsmenus.pas	(working copy)
@@ -395,8 +395,7 @@
 
 procedure TCocoaMenuItem_Quit.lclItemSelected(sender: id);
 begin
-  // Should be used instead of Application.Terminate to allow events to be sent, see bug 32148
-  Application.MainForm.Close;
+  CocoaWidgetSet.QuitApp;
 end;
 
 { TCocoaWSMenu }
quitapp.patch (4,330 bytes)

Zoë Peterson

2019-01-03 00:22

reporter  

quitapp-NoMainForm.zip (66,022 bytes)

Zoë Peterson

2019-01-03 00:22

reporter  

quitapp-Shutdown.zip (67,744 bytes)

Zoë Peterson

2019-05-16 18:14

reporter   ~0116220

Last edited: 2019-05-16 18:14

View 2 revisions

This is a new, smaller version of the patch that doesn't include the TCocoaWidgetset.OnQuitApp behavior.

This fixes the issue where on system shutdown/logout, or if another application sends us a quit event, none of MainForm.OnCloseQuery & OnClose, Application.OnQueryEndSession & OnEndSession, or the app finalization sections are called.

It can be tested with the existing quitapp-Shutdown.zip sample project.



cocoa-shutdown.patch (2,753 bytes)
diff --git a/lcl/interfaces/cocoa/cocoaint.pas b/lcl/interfaces/cocoa/cocoaint.pas
index ac6e600e55..69da9addbf 100644
--- a/lcl/interfaces/cocoa/cocoaint.pas
+++ b/lcl/interfaces/cocoa/cocoaint.pas
@@ -58,6 +58,8 @@ type
     procedure applicationDidBecomeActive(notification: NSNotification);
     procedure applicationDidResignActive(notification: NSNotification);
     procedure applicationDidChangeScreenParameters(notification: NSNotification);
+    procedure applicationWillFinishLaunching(notification: NSNotification);
+    procedure handleQuitAppEvent_withReplyEvent(event: NSAppleEventDescriptor; replyEvent: NSAppleEventDescriptor); message 'handleQuitAppEvent:withReplyEvent:';
   end;
 
   { TCocoaApplication }
diff --git a/lcl/interfaces/cocoa/cocoaobject.inc b/lcl/interfaces/cocoa/cocoaobject.inc
index 5a17d799d0..2ea07501dc 100644
--- a/lcl/interfaces/cocoa/cocoaobject.inc
+++ b/lcl/interfaces/cocoa/cocoaobject.inc
@@ -592,6 +592,43 @@ begin
   Screen.UpdateScreen;
 end;
 
+procedure TAppDelegate.applicationWillFinishLaunching(notification: NSNotification);
+begin
+  NSAppleEventManager.sharedAppleEventManager.setEventHandler_andSelector_forEventClass_andEventID(
+    Self, ObjCSelector('handleQuitAppEvent:withReplyEvent:'), kCoreEventClass,
+    kAEQuitApplication);
+end;
+
+procedure TAppDelegate.handleQuitAppEvent_withReplyEvent(event: NSAppleEventDescriptor; replyEvent: NSAppleEventDescriptor);
+{ Capture "Quit Application" Apple Events, either from system shutdown/logout
+  or sent by another application.  Don't use [applicationShouldTerminate:]
+  because that terminates the app immediately after [applicationWillTerminate:]
+  returns, so there's no chance to run finalization blocks }
+var
+  Cancel: Boolean;
+  Reason: NSAppleEventDescriptor;
+begin
+  Cancel := False;
+  // Check if it's a system-wide event
+  Reason := event.attributeDescriptorForKeyword(kEventParamReason);
+  if (Reason <> nil) and
+     ((Reason.typeCodeValue = kAEQuitAll) or
+      (reason.typeCodeValue = kAEReallyLogOut) or
+      (reason.typeCodeValue = kAERestart) or
+      (reason.typeCodeValue = kAEShutDown)) then
+  begin
+    Application.IntfQueryEndSession(Cancel);
+    if not Cancel then
+      Application.IntfEndSession;
+  end;
+  // Try to quit
+  if not Cancel then
+    Application.MainForm.Close;
+  // Let caller know if the shutdown was cancelled
+  if (not Application.Terminated) and (replyEvent.descriptorType <> typeNull) then
+    replyEvent.setParamDescriptor_forKeyword(NSAppleEventDescriptor.descriptorWithInt32(userCanceledErr), keyErrorNumber);
+end;
+
 {------------------------------------------------------------------------------
   Method:  TCocoaWidgetSet.RawImage_DescriptionFromCocoaBitmap
 
cocoa-shutdown.patch (2,753 bytes)

Dmitry Boyarintsev

2019-05-16 20:30

developer   ~0116221

thanks for the patch, applied
please test and close if ok

Issue History

Date Modified Username Field Change
2019-01-03 00:22 Zoë Peterson New Issue
2019-01-03 00:22 Zoë Peterson File Added: quitapp.patch
2019-01-03 00:22 Zoë Peterson File Added: quitapp-NoMainForm.zip
2019-01-03 00:22 Zoë Peterson File Added: quitapp-Shutdown.zip
2019-05-16 18:14 Zoë Peterson File Added: cocoa-shutdown.patch
2019-05-16 18:14 Zoë Peterson Note Added: 0116220
2019-05-16 18:14 Zoë Peterson Note Edited: 0116220 View Revisions
2019-05-16 20:30 Dmitry Boyarintsev Assigned To => Dmitry Boyarintsev
2019-05-16 20:30 Dmitry Boyarintsev Status new => resolved
2019-05-16 20:30 Dmitry Boyarintsev Resolution open => fixed
2019-05-16 20:30 Dmitry Boyarintsev Fixed in Revision => 61230
2019-05-16 20:30 Dmitry Boyarintsev LazTarget => -
2019-05-16 20:30 Dmitry Boyarintsev Widgetset Cocoa => Cocoa
2019-05-16 20:30 Dmitry Boyarintsev Note Added: 0116221
2019-05-23 19:04 Zoë Peterson Status resolved => closed