GNOME Bugzilla – Bug 626499
GtkClipboard unnotified on change of OS X pasteboard owner
Last modified: 2013-01-15 11:02:45 UTC
Created attachment 167475 [details] [review] Adds debugging and fixes intialisation of clear_func gtk_clipboard_set_with_data() may be passed a function to be invoked when "the clipboard contents are set again". This works as expected under win32 but not under OS X. OS X provides this functionality via the pasteboardChangedOwner: method of the NSPasteboardOwner informal protocol (1), and gtkclipboard-quartz.c appears to supply a conforming object. However GTK is not notified when its posted clipboard targets are replaced by those of another application - say by copying some text in TextEdit. The attached test program + patch illustrates the problem: GTK+ version 2.20.1 Connecting GtkClipboard::owner-change signal Posting 1 targets: 0: image/png Gtk-Message: gtkclipboard-quartz.c:144GtkClipboardOwner::initWithClipboard() Gtk-Message: gtkclipboard-quartz.c:154 ... clear func is 0x2854 Calling gtk_clipboard_set_with_data... Success [ copy text in TextEdit here ] [ expected "GtkClipboardOwner::pasteboardChangedOwner()" to be printed ] On win32, this is: GTK+ version 2.18.7 Connecting GtkClipboard::owner-change signal Posting 1 targets: 0: image/png Calling gtk_clipboard_set_with_data... Success [ copy some text in another process ] GtkClipboardClearFunc callback invoked! [ <-- correct notification ] ^C (Note that GtkClipboardOwner did not initialise its clear_func correctly; this is fixed in the attached patch) (1) see http://developer.apple.com/mac/library/DOCUMENTATION/AppKit/Reference/NSPasteboardOwner_Protocol/Reference/Reference.html)
Created attachment 167476 [details] Used to demonstrate the problem
The test program was run under OS X 10.4, and the same problem looks to occur under OS X 10.5.
I can replicate this on 10.6 as well, and with a current Git master build of Gtk+-3.0. It appears that GtkPasteboardOwner pasteboardChangedOwner: isn't getting called when one copies something from another application to the pasteboard.
Okay, this is very confusing :) I spent a good deal of time being completely puzzled why the psateboardChangedOwner: message didn't come in. Basically, this message is not sent instantaneously. As far as I can see, it is only sent the next time the same process calls declareTypes. I suspect this is to not wake up the process in between immediately when some other process modifies the clipboard, but rather delay the cleanup work until the process is active anyway. So this means: GTK+ process sends declareTypes: and thus sets owner Some other process sets the clipboard. (More other processes set the clipboard). GTK+ process sends declareTypes: and thus sets owner; at this point we receive the changed owner message. The clear function is thus called delayed. This more or less fits with the current API description of clear_func, it is to be notifed that get_func will not subsequently be called. On OS X, we cannot rely on clear_func to be notified exactly when the clipboard ownership is lost. To call the clear function sooner, we can use the trick of monitoring the pasteboard' change count and check for changes in this once the application becomes active/deactive and call the clear function at that point. Requires a clean way to monitor application (de)activation in gtkclipboard (with no handle to a window) though.
(In reply to comment #0) > Created an attachment (id=167475) [details] [review] > Adds debugging and fixes intialisation of clear_func I have committed the bug fixes from this attachment: (i) I rewrote the initialization of clear_func, to just use the pointers from the clipboard pointer and not make copies. User_data was not initialized either. (ii) Delay initialization of owner so it is not leaked.
In addition to comment 4, another thing that needs fixing is bookkeeping which types have been requested through pasteboard:provideDataForType:. When all of the possible types have been requested, the pasteboardChangedOwner: will not be sent.
Created attachment 230358 [details] [review] Patch that seems to completel fix it for me Attached patch keeps track of [pasteboard changeCount] which is an indication of owner change, and drops clipboard ownership when it changes.
*** Bug 686279 has been marked as a duplicate of this bug. ***
Review of attachment 230358 [details] [review]: ::: gtk/gtkclipboard-quartz.c @@ +127,3 @@ - (void)pasteboardChangedOwner:(NSPasteboard *)sender { + clipboard_unset (clipboard); This is problematic, because this method will be called when we call declareTypes: again to set new data (so in a subsequent copy operation). See function gtk_clipboard_set_contents(), what happens is: - the clipboard is being set up, new target list is being created amongst other things. - then we call declareTypes, at which point the old clipboard owner is sent pasteboardChangedOwner - pasteboardChangedOwner will call clipboard_unset(), which will unset the JUST created target_list. This can be reproduced as follows: copy in GIMP twice in a row, then try Clipboard Viewer. There will be targets, but all targets are empty. @@ +373,2 @@ [clipboard->pasteboard declareTypes:[types allObjects] owner:owner]; + clipboard->change_count = [clipboard->pasteboard changeCount]; declareTypes: will return the new change count, so the separate "changeCount" message is unnecessary. @@ +422,3 @@ + { + clipboard_unset (clipboard); + clipboard->change_count = [clipboard->pasteboard changeCount]; This will unset the clipboard on every change, even when we did not lose ownership. Is it an idea to only update changeCount when we declare new types (because we then become owner) and only unset the clipboard when we lose ownership? Or will that not work reliably?
Created attachment 230367 [details] [review] Updated patch Patch that fixes the issue kris pointed out, adds a comment. The last issue mentioned is not fixed because that's exactly why we track changeCount.
(In reply to comment #10) > Created an attachment (id=230367) [details] [review] > Updated patch > > Patch that fixes the issue kris pointed out, adds a comment. A bit of a hack, but it will do ;) > The last issue mentioned is not fixed because that's exactly > why we track changeCount. The last issue was kind of related to the first one, we might be calling clipboard_unset() when unnecessary. But it shouldn't hurt. By the way, do you think gtk_clipboard_clear() needs to call clipboard_unset() too?
> By the way, do you think gtk_clipboard_clear() needs to call clipboard_unset() > too? Probably, but there is not really a reasonable use case for it :) GIMP does call it, but only in paranoia code.
Apparently, the latest patch still crashes testoffscreen. Open the app, wait for 20 seconds and close it => segfault.
Details: testoffscreen(52451,0x7fff7ad2d960) malloc: *** error for object 0x10218c9f0: po nter being freed was not allocated
+ Trace 231249
Created attachment 230456 [details] [review] Call clipboard_unset() also from gtk_clipboard_unset() This seems to completely fix the crash in testoffscreen.
Created attachment 230643 [details] [review] The right patch this time
Fixed in master, gtk-3-6 and gtk-2-24: commit f08fc1274141a0e631687de12246409881ec2454 Author: Michael Natterer <mitch@gimp.org> Date: Tue Dec 4 14:31:13 2012 +0100 Bug 626499 - GtkClipboard unnotified on change of OS X pasteboard owner pasteboardChangedOwner is not called as reliably as we'd want to get it, so keep track of [pasteboard changeCount] and drop clipboard ownership when a change happened. Also better unset the clipboard content redundantly in a few places rather than missing one, and reorder the code in gtk_clipboard_set_contents() so that the new aggressive unsetting won't unset the clipboard under our feet when we call [pasteboard declareTypes]. (cherry picked from commit f2b74db5dcbd28a1e27431f413c66d2a5d50b2bd) gtk/gtkclipboard-quartz.c | 63 +++++++++++++++++++++++++++++--------------- 1 files changed, 41 insertions(+), 22 deletions(-)
Tip of 2-24 branch (commit e3e055f8551ac8ee033f361261c849c612554184 ) still crashes on shutdown on macos. Steps to repro: 1) Launch gtk-demo 2) Select text in the 'Info' pane and copy it to the clipboard 3) Flip open a chrome window (or any other application). Copy text to the clipboard in that app a few times. Maybe copy different things too. 4) Go back to gtk-demo and just try to quit it Result: Crash!
+ Trace 231305
We crash when invoking clipboard->get_func as it is NULL because we have reset/cleared the clipboard already.
And re-fixed. This is only in gtk-2-24. Let's verify that this now works nicely before merging the entire store stuff to gtk3, it's completely missing there. commit f1c105b94fc3c3572a234c93c47252a3ff82218b Author: Michael Natterer <mitch@gimp.org> Date: Thu Dec 20 23:37:06 2012 +0100 quartz: don't call a NULL get_func() in gtk_clipboard_store() Assume the clipboard is not set and bail out silently (bug #626499). gtk/gtkclipboard-quartz.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Unfortunately this is still broken :)
+ Trace 231378
The issue is that when we are iterating over all the targets in gtk_clipboard_store we can end up calling 'clipboard_unset'. The result is that if there is more than one target we invoke clipboard_unset which clears the get_func when processing the first target and then invoke a null get_func during the second iteration. The trace above is from gdb during the first iteration of this loop: 1062 for (i = 0; i < n_targets; i++) 1063 { 1064 GtkSelectionData selection_data; 1065 1066 memset (&selection_data, 0, sizeof (GtkSelectionData)); 1067 1068 selection_data.selection = clipboard->selection; 1069 selection_data.target = gdk_atom_intern_static_string (targets[i].target); 1070 selection_data.display = gdk_display_get_default (); 1071 selection_data.length = -1; 1072 1073 clipboard->get_func (clipboard, &selection_data, 1074 targets[i].info, clipboard->user_data); 1075 1076 if (selection_data.length >= 0) 1077 _gtk_quartz_set_selection_data_for_pasteboard (clipboard->pasteboard, 1078 &selection_data); 1079 1080 g_free (selection_data.data); 1081 } The repro is the same as in comment 18.
And re-fixed :) commit bc3f1893aa26761c0009ddc993b48623bcfbe4ed Author: Michael Natterer <mitch@lanedo.com> Date: Tue Jan 15 11:48:23 2013 +0100 quartz: really don't call a NULL function in gtk_clipboard_store() Need to check targets and get_func in each loop iteration because calling get_func the fist time might do whatever to the clipboard. Re-fixes bug #626499. Also free the target table after we're done. gtk/gtkclipboard-quartz.c | 10 ++++++++++ 1 file changed, 10 insertions(+)