Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Retry fetching clipboard contents when it initially fails #6443

Merged
merged 2 commits into from
Sep 23, 2023
Merged

Retry fetching clipboard contents when it initially fails #6443

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b01c99
Retry fetching clipboard contents when it initially fails
matthiasblaesing Sep 8, 2023
0e226f2
Cleanup warnings/refactoring hints in NbClipboard
matthiasblaesing Sep 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 82 additions & 51 deletions platform/o.n.bootstrap/src/org/netbeans/NbClipboard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,23 @@
import org.openide.util.Utilities;
import org.openide.util.datatransfer.ExClipboard;

@org.openide.util.lookup.ServiceProviders({@org.openide.util.lookup.ServiceProvider(service=java.awt.datatransfer.Clipboard.class), @org.openide.util.lookup.ServiceProvider(service=org.openide.util.datatransfer.ExClipboard.class)})
@org.openide.util.lookup.ServiceProviders({
@org.openide.util.lookup.ServiceProvider(service=java.awt.datatransfer.Clipboard.class),
@org.openide.util.lookup.ServiceProvider(service=org.openide.util.datatransfer.ExClipboard.class)
})
public final class NbClipboard extends ExClipboard
implements LookupListener, FlavorListener, AWTEventListener
{
private static final Logger log = Logger.getLogger(NbClipboard.class.getName());
private ThreadLocal<Boolean> FIRING = new ThreadLocal<Boolean>();
private final ThreadLocal<Boolean> FIRING = new ThreadLocal<>();
private Clipboard systemClipboard;
private ExClipboard.Convertor[] convertors;
private Lookup.Result<ExClipboard.Convertor> result;
final boolean slowSystemClipboard;
private Transferable last;
private long lastWindowActivated;
private long lastWindowDeactivated;
private Reference<Object> lastWindowDeactivatedSource = new WeakReference<Object>(null);
private Reference<Object> lastWindowDeactivatedSource = new WeakReference<>(null);
private volatile Task setContentsTask = Task.EMPTY;
private volatile Task getContentsTask = Task.EMPTY;
private boolean anyWindowIsActivated = true;
Expand All @@ -68,7 +71,8 @@ public NbClipboard() {
//for unit testing
this( Toolkit.getDefaultToolkit().getSystemClipboard() );
}


@SuppressWarnings("LeakingThisInConstructor")
NbClipboard( Clipboard systemClipboard ) {
super("NBClipboard"); // NOI18N
this.systemClipboard = systemClipboard;
Expand All @@ -80,7 +84,7 @@ public NbClipboard() {

resultChanged(null);

if (System.getProperty("netbeans.slow.system.clipboard.hack") != null) {
if (System.getProperty("netbeans.slow.system.clipboard.hack") != null) { // NOI18N
slowSystemClipboard = Boolean.getBoolean("netbeans.slow.system.clipboard.hack"); // NOI18N
} else if (Utilities.isMac()) {
slowSystemClipboard = false;
Expand All @@ -100,11 +104,14 @@ public NbClipboard() {
this, AWTEvent.WINDOW_EVENT_MASK);
}
}


@Override
@SuppressWarnings("ReturnOfCollectionOrArrayField")
protected synchronized ExClipboard.Convertor[] getConvertors () {
return convertors;
}

@Override
public synchronized void resultChanged(LookupEvent ev) {
Collection<? extends ExClipboard.Convertor> c = result.allInstances();
ExClipboard.Convertor[] temp = new ExClipboard.Convertor[c.size()];
Expand All @@ -124,7 +131,7 @@ public synchronized void resultChanged(LookupEvent ev) {
// accesses the system clipboard directly then we don't see these changes.
//
// The other problem is an AWT bug
//
//
// http://developer.java.sun.com/developer/bugParade/bugs/4818143.html
//
// sun.awt.datatransfer.ClipboardTransferable.getClipboardData() can hang
Expand All @@ -136,6 +143,7 @@ public synchronized void resultChanged(LookupEvent ev) {
private static final RequestProcessor RP = new RequestProcessor("System clipboard synchronizer"); // NOI18N

@Override
@SuppressWarnings("AssignmentToMethodParameter")
public void setContents(Transferable contents, ClipboardOwner owner) {
synchronized (this) {
// XXX(-dstrupl) the following line might lead to a double converted
Expand All @@ -162,12 +170,8 @@ public void setContents(Transferable contents, ClipboardOwner owner) {
this.contents = contents;

if (oldOwner != null && oldOwner != owner) {
EventQueue.invokeLater(new Runnable() {

@Override
public void run() {
oldOwner.lostOwnership(NbClipboard.this, oldContents);
}
EventQueue.invokeLater(() -> {
oldOwner.lostOwnership(NbClipboard.this, oldContents);
});
}
} else {
Expand All @@ -189,12 +193,11 @@ public Transferable getContents(Object requestor) {
// The purpose of lastWindowActivated+100 is to ignore calls
// which immediatelly follow WINDOW_ACTIVATED event.
// This is workaround of JDK bug described in issue 41098.
long curr = System.currentTimeMillis();
int waitTime = Integer.getInteger("sun.awt.datatransfer.timeout", 1000); // NOI18N
if (waitTime > 0 && !Boolean.TRUE.equals(FIRING.get())) {
boolean ok = scheduleGetFromSystemClipboard(false).waitFinished (waitTime);
if (!ok) {
log.log(Level.FINE, "Time out waiting for sync with system clipboard for {0} ms", waitTime);
log.log(Level.FINE, "Time out waiting for sync with system clipboard for {0} ms", waitTime); // NOI18N
}
}
prev = super.getContents (requestor);
Expand Down Expand Up @@ -252,18 +255,19 @@ public synchronized void addFlavorListener(FlavorListener listener) {
Boolean prev = FIRING.get();
try {
FIRING.set(true);
super.addFlavorListener(listener);
super.addFlavorListener(listener);
} finally {
FIRING.set(prev);
}
}



private void scheduleSetContents(final Transferable cnts, final ClipboardOwner ownr, int delay) {
setContentsTask = RP.post(new SetContents(cnts, ownr), delay);
}


@SuppressWarnings("NestedAssignment")
private Task scheduleGetFromSystemClipboard(boolean notify) {
return getContentsTask = RP.post(new GetContents(notify));
}
Expand All @@ -274,39 +278,37 @@ final void waitFinished () {
setContentsTask.waitFinished ();
getContentsTask.waitFinished ();
}

final void activateWindowHack (boolean reschedule) {
// if WINDOW_DEACTIVATED is followed immediatelly with
// WINDOW_ACTIVATED then it is JDK bug described in
// WINDOW_ACTIVATED then it is JDK bug described in
// issue 41098.
lastWindowActivated = System.currentTimeMillis();
if (reschedule) {
scheduleGetFromSystemClipboard(true);
}
}

private void logFlavors (Transferable trans, Level level, boolean content) {
if (trans == null) {
log.log (level, " no clipboard contents");
log.log (level, " no clipboard contents"); // NOI18N
}
else {
if (content) {
java.awt.datatransfer.DataFlavor[] arr = trans.getTransferDataFlavors();
StringBuilder sb = new StringBuilder();
for (int i = 0; i < arr.length; i++) {
sb.append(" ").append(i).append(" = ").append(arr[i]);
sb.append(" ").append(i).append(" = ").append(arr[i]); // NOI18N
try {
sb.append(" contains: ").append(trans.getTransferData(arr[i]));
} catch (UnsupportedFlavorException ex) {
log.log(level, "Can't convert to " + arr[i], ex);
} catch (IOException ex) {
log.log(level, "Can't convert to " + arr[i], ex);
sb.append(" contains: ").append(trans.getTransferData(arr[i])); // NOI18N
} catch (UnsupportedFlavorException | IOException ex) {
log.log(level, "Can't convert to " + arr[i], ex); // NOI18N
}
sb.append("\n");
}
log.log (level, sb.toString());
} else {
log.log (level, " clipboard contains data");
log.log (level, " clipboard contains data"); // NOI18N
}
}
}
Expand Down Expand Up @@ -339,10 +341,10 @@ public void eventDispatched(AWTEvent ev) {

if (ev.getID() == WindowEvent.WINDOW_DEACTIVATED) {
lastWindowDeactivated = System.currentTimeMillis();
lastWindowDeactivatedSource = new WeakReference<Object>(ev.getSource());
lastWindowDeactivatedSource = new WeakReference<>(ev.getSource());
anyWindowIsActivated = false;
if( Utilities.isWindows() ) {
//#247585 - even listening to clipboard changes when the window isn't active
//#247585 - even listening to clipboard changes when the window isn't active
//may throw a MS Windows error as the 'clipboard copy' action doesn't have enough time to finish
systemClipboard.removeFlavorListener(this);
}
Expand Down Expand Up @@ -377,40 +379,41 @@ private synchronized void superSetContents(Transferable t, ClipboardOwner o) {
this.contents = t;

if (oldOwner != null && oldOwner != owner) {
EventQueue.invokeLater(new Runnable() {
@Override
public void run() {
oldOwner.lostOwnership(NbClipboard.this, oldContents);
}
EventQueue.invokeLater(() -> {
oldOwner.lostOwnership(NbClipboard.this, oldContents);
});
}
}

/** Transferable that logs operations on itself.
*/
private final class LoggableTransferable implements Transferable {
private Transferable delegate;
private final Transferable delegate;

public LoggableTransferable (Transferable delegate) {
this.delegate = delegate;
}

@Override
public Object getTransferData (DataFlavor flavor) throws UnsupportedFlavorException, java.io.IOException {
log.log (Level.FINE, "Request for flavor: " + flavor); // NOI18N
log.log (Level.FINE, "Request for flavor: {0}", flavor); // NOI18N
Object res = delegate.getTransferData (flavor);
log.log (Level.FINE, "Returning value: " + res); // NOI18N
log.log (Level.FINE, "Returning value: {0}", res); // NOI18N
return res;
}


@Override
public DataFlavor[] getTransferDataFlavors () {
return delegate.getTransferDataFlavors ();
}


@Override
public boolean isDataFlavorSupported (DataFlavor flavor) {
boolean res = delegate.isDataFlavorSupported (flavor);
log.log (Level.FINE, "isDataFlavorSupported: " + flavor + " result: " + res); // NOI18N
log.log (Level.FINE, "isDataFlavorSupported: {0} result: {1}", new Object[]{flavor, res}); // NOI18N
return res;
}

}

private final class GetContents implements Runnable {
Expand All @@ -419,12 +422,35 @@ private final class GetContents implements Runnable {
public GetContents(boolean notify) {
this.notify = notify;
}

@Override
@SuppressWarnings("SleepWhileInLoop")
public void run() {
log.fine("Running update");
log.fine("Running update"); // NOI18N
try {
Transferable transferable = systemClipboard.getContents(this);
Transferable transferable = null;
// There can be a race between multiple applications accessing
// the clipboard. If access can't be optained directly, retry
// for a maximum of 1s. This is called from the requestprocessor
mbien marked this conversation as resolved.
Show resolved Hide resolved
// that is used because accessing the clipboard can block
// indefinitely. Running the access loop here is deemed similar
// in nature.
final int MAX_TRIES = 50;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to handle this issue differently in GetContents from SetContents? I think without good reason they should handle this in the same way, whether it be this way or that way being open to question.

https://github.com/apache/netbeans/blob/master/platform/o.n.bootstrap/src/org/netbeans/NbClipboard.java#L458

The pro and con of rescheduling rather than looping is that anything waiting on the task won't wait for the rescheduled one.

final long start = System.currentTimeMillis();
for (int i = 0; i < MAX_TRIES; i++) {
try {
transferable = systemClipboard.getContents(this);
break;
} catch (IllegalStateException ex) {
// Throw exception if retries failed
eirikbakke marked this conversation as resolved.
Show resolved Hide resolved
if (i == (MAX_TRIES - 1) || (System.currentTimeMillis() - start) > 980L) {
throw ex;
} else {
log.log(Level.INFO, "systemClipboard#getContents threw IllegalStateException (try: {0})", i + 1); // NOI18N
}
Thread.sleep(20); // Give system time to settle
eirikbakke marked this conversation as resolved.
Show resolved Hide resolved
}
}
superSetContents(transferable, null);
if (log.isLoggable (Level.FINE)) {
log.log (Level.FINE, "internal clipboard updated:"); // NOI18N
Expand All @@ -437,7 +463,8 @@ public void run() {
catch (ThreadDeath ex) {
throw ex;
}
catch (Throwable ignore) {
catch (InterruptedException | RuntimeException ex) {
log.log(Level.INFO, "systemClipboard not available", ex); // NOI18N
}
}
}
Expand All @@ -457,7 +484,11 @@ public void run() {
systemClipboard.setContents(cnts, ownr);
} catch (IllegalStateException e) {
//#139616
log.log(Level.FINE, "systemClipboard not available", e); // NOI18N
if(log.isLoggable(Level.FINE)) {
log.log(Level.FINE, "systemClipboard not available", e); // NOI18N
} else {
log.log(Level.INFO, "systemClipboard#setContents threw IllegalStateException"); // NOI18N
}
scheduleSetContents(cnts, ownr, 100);
return;
}
Expand Down
Loading