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.

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

8272756: Remove unnecessary explicit initialization of volatile variables in java.desktop #5197

Closed
wants to merge 3 commits into from
Closed
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -119,7 +119,7 @@ public void updateCursorImmediately() {
}

// SwingNode
private volatile long overriddenWindowHandle = 0L;
private volatile long overriddenWindowHandle;

@Override
public void overrideWindowHandle(final long handle) {
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -112,7 +112,7 @@ public enum PeerType {
private volatile int windowState = Frame.NORMAL;

// check that the mouse is over the window
private volatile boolean isMouseOver = false;
private volatile boolean isMouseOver;

// A peer where the last mouse event came to. Used by cursor manager to
// find the component under cursor
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -33,7 +33,7 @@

public class CCheckboxMenuItem extends CMenuItem implements CheckboxMenuItemPeer {
volatile boolean fAutoToggle = true;
volatile boolean fIsIndeterminate = false;
volatile boolean fIsIndeterminate;

private native void nativeSetState(long modelPtr, boolean state);
private native void nativeSetIsCheckbox(long modelPtr);
@@ -50,8 +50,8 @@ public class CPlatformEmbeddedFrame implements PlatformWindow {
private LWWindowPeer peer;
private CEmbeddedFrame target;

private volatile int screenX = 0;
private volatile int screenY = 0;
private volatile int screenX;
private volatile int screenY;

@Override // PlatformWindow
public void initialize(Window target, final LWWindowPeer peer, PlatformWindow owner) {
@@ -56,7 +56,7 @@ private static class Lock {}
/**
* Animation stage.
*/
private volatile int currentIcon = 0;
private volatile int currentIcon;

/* -1 - uninitialized.
* 0 - 16x16
@@ -56,12 +56,12 @@ public class TIFFIFD extends TIFFDirectory {
// A set of tag numbers corresponding to tags essential to decoding
// the image and metadata required to interpret its samples.
//
private static volatile Set<Integer> essentialTags = null;
private static volatile Set<Integer> essentialTags;

private static void initializeEssentialTags() {
Set<Integer> tags = essentialTags;
if (tags == null) {
essentialTags = tags = Set.of(
essentialTags = Set.of(
Copy link
Member

@mrserb mrserb Aug 20, 2021

Choose a reason for hiding this comment

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

What is the purpose of the local tags here?

Copy link
Contributor Author

@stsypanov stsypanov Aug 23, 2021

Choose a reason for hiding this comment

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

Looks like there's no purpose, tags is not used after assignment

Copy link
Member

@aivanov-jdk aivanov-jdk Aug 23, 2021

Choose a reason for hiding this comment

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

Maybe it's kind of a protection from a race. Yet it's possible either way: another thread could see essentialTags == null before this one initialises the field.

Copy link
Member

@mrserb mrserb Aug 23, 2021

Choose a reason for hiding this comment

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

I think we can drop it completely.

Copy link
Member

@aivanov-jdk aivanov-jdk Aug 23, 2021

Choose a reason for hiding this comment

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

Agree.

Copy link
Member

@mrserb mrserb Sep 26, 2021

Choose a reason for hiding this comment

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

I still suggest to remove the local tag completely.

Copy link
Member

@mrserb mrserb Oct 8, 2021

Choose a reason for hiding this comment

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

@jayathirthrao @stsypanov Looks like this was not addressed? It will be good to include it in some future cleanup.

Copy link
Member

@aivanov-jdk aivanov-jdk Oct 8, 2021

Choose a reason for hiding this comment

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

As far as I can see, it was. Line 64:

            essentialTags = Set.of(

And it shows as such in the diff of the commit.

Copy link
Member

@mrserb mrserb Oct 8, 2021

Choose a reason for hiding this comment

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

That was the usage of the local var, the current thread was about the local var itself, it is still there.

Copy link
Member

@aivanov-jdk aivanov-jdk Oct 8, 2021

Choose a reason for hiding this comment

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

Good catch!
I missed that the local variable is still there and now it's unused, so an IDE should issue a warning about it.

BaselineTIFFTagSet.TAG_BITS_PER_SAMPLE,
BaselineTIFFTagSet.TAG_COLOR_MAP,
BaselineTIFFTagSet.TAG_COMPRESSION,
@@ -361,13 +361,13 @@ private static class DirectDL extends AbstractDataLine implements EventDispatche
protected final int deviceID;
protected long id;
protected int waitTime;
protected volatile boolean flushing = false;
protected volatile boolean flushing;
protected final boolean isSource; // true for SourceDataLine, false for TargetDataLine
protected volatile long bytePosition;
protected volatile boolean doIO = false; // true in between start() and stop() calls
protected volatile boolean stoppedWritten = false; // true if a write occurred in stopped state
protected volatile boolean drained = false; // set to true when drain function returns, set to false in write()
protected boolean monitoring = false;
protected volatile boolean doIO; // true in between start() and stop() calls
protected volatile boolean stoppedWritten; // true if a write occurred in stopped state
protected volatile boolean drained; // set to true when drain function returns, set to false in write()
protected boolean monitoring;

// if native needs to manually swap samples/convert sign, this
// is set to the framesize
@@ -379,7 +379,7 @@ private static class DirectDL extends AbstractDataLine implements EventDispatche
private final Balance balanceControl = new Balance();
private final Pan panControl = new Pan();
private float leftGain, rightGain;
protected volatile boolean noService = false; // do not run the nService method
protected volatile boolean noService; // do not run the nService method

// Guards all native calls.
protected final Object lockNative = new Object();
@@ -975,7 +975,7 @@ private static final class DirectClip extends DirectDL
implements Clip, Runnable, AutoClosingClip {

private volatile Thread thread;
private volatile byte[] audioData = null;
private volatile byte[] audioData;
private volatile int frameSize; // size of one frame in bytes
private volatile int m_lengthInFrames;
private volatile int loopCount;
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2007, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2007, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -37,8 +37,8 @@
*/
public final class SoftAudioPusher implements Runnable {

private volatile boolean active = false;
private SourceDataLine sourceDataLine = null;
private volatile boolean active;
private SourceDataLine sourceDataLine;
private Thread audiothread;
private final AudioInputStream ais;
private final byte[] buffer;
@@ -76,7 +76,7 @@ protected static final class WeakAudioStream extends InputStream
public SoftAudioPusher pusher = null;
public AudioInputStream jitter_stream = null;
public SourceDataLine sourceDataLine = null;
public volatile long silent_samples = 0;
public volatile long silent_samples;
private int framesize = 0;
private final WeakReference<AudioInputStream> weak_stream_link;
private final AudioFloatConverter converter;
@@ -395,7 +395,7 @@ public abstract class Component implements ImageObserver, MenuContainer,
* @see #validate
* @see #invalidate
*/
private volatile boolean valid = false;
private volatile boolean valid;

/**
* The {@code DropTarget} associated with this component.
@@ -9340,7 +9340,7 @@ protected AccessibleAWTComponent() {
* to add/remove ComponentListener and FocusListener to track
* target Component's state.
*/
private transient volatile int propertyListenersCount = 0;
private transient volatile int propertyListenersCount;

/**
* A component listener to track show/hide/resize events
@@ -3861,7 +3861,7 @@ public Accessible getAccessibleAt(Point p) {
* Number of PropertyChangeListener objects registered. It's used
* to add/remove ContainerListener to track target Container's state.
*/
private transient volatile int propertyListenersCount = 0;
private transient volatile int propertyListenersCount;

/**
* The handler to fire {@code PropertyChange}
@@ -301,7 +301,7 @@ public static enum ModalExclusionType {
* @see #hideAndDisposeHandler()
* @see #shouldBlock()
*/
transient volatile boolean isInHide = false;
transient volatile boolean isInHide;

/*
* Indicates that this dialog is being disposed. This flag is set to true at
@@ -312,7 +312,7 @@ public static enum ModalExclusionType {
* @see #hideAndDisposeHandler()
* @see #doDispose()
*/
transient volatile boolean isInDispose = false;
transient volatile boolean isInDispose;

private static final String base = "dialog";
private static int nameCounter = 0;
@@ -404,8 +404,8 @@ public static enum Type {
* These fields are initialized in the native peer code
* or via AWTAccessor's WindowAccessor.
*/
private transient volatile int securityWarningWidth = 0;
private transient volatile int securityWarningHeight = 0;
private transient volatile int securityWarningWidth;
private transient volatile int securityWarningHeight;

static {
/* ensure that the necessary native libraries are loaded */
@@ -116,7 +116,7 @@ public void dispose(InvocationEvent invocationEvent) {
* @see #isDispatched
* @since 1.7
*/
private volatile boolean dispatched = false;
private volatile boolean dispatched;

/**
* Set to true if dispatch() catches Throwable and stores it in the
@@ -1364,7 +1364,7 @@ private Range rangeForCodePoint(final int codepoint) {

// use a binary search with a cache

private transient volatile int stCache = 0;
private transient volatile int stCache;

private boolean isStrongDirectional(char c) {
int cachedIndex = stCache;
@@ -3742,7 +3742,7 @@ protected AccessibleJComponent() {
* to add/remove ContainerListener and FocusListener to track
* target JComponent's state
*/
private transient volatile int propertyListenersCount = 0;
private transient volatile int propertyListenersCount;

/**
* This field duplicates the function of the accessibleAWTFocusHandler field
@@ -161,7 +161,7 @@ public static Set<AppContext> getAppContexts() {
contained in another AppContext. It is implicitly created for
standalone apps only (i.e. not applets)
*/
private static volatile AppContext mainAppContext = null;
private static volatile AppContext mainAppContext;

private static class GetAppContextLock {};
private static final Object getAppContextLock = new GetAppContextLock();
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -71,7 +71,7 @@ public abstract class SunClipboard extends Clipboard
* A number of {@code FlavorListener}s currently registered
* on this clipboard across all {@code AppContext}s.
*/
private volatile int numberOfFlavorListeners = 0;
private volatile int numberOfFlavorListeners;

/**
* A set of {@code DataFlavor}s that is available on this clipboard. It is
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -74,8 +74,8 @@ public abstract class SunDragSourceContextPeer implements DragSourceContextPeer
private DragSourceContext dragSourceContext;
private int sourceActions;

private static volatile boolean dragDropInProgress = false;
private static volatile boolean discardingMouseEvents = false;
private static volatile boolean dragDropInProgress;
private static volatile boolean discardingMouseEvents;

/*
* dispatch constants
@@ -184,7 +184,7 @@ private static void clearDeferredRecords() {
/*
* Set to indicate the queue is presently being polled.
*/
public static volatile boolean pollingQueue = false;
public static volatile boolean pollingQueue;

/*
* The pollRemove() method is called back from a dispose method
@@ -334,7 +334,7 @@ void dump() {
static final class RendererStatsHolder {

// singleton
private static volatile RendererStatsHolder SINGLETON = null;
private static volatile RendererStatsHolder SINGLETON;

static synchronized RendererStatsHolder getInstance() {
if (SINGLETON == null) {
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -40,7 +40,7 @@ final class GtkFileDialogPeer extends XDialogPeer implements FileDialogPeer {
private final FileDialog fd;

// A pointer to the native GTK FileChooser widget
private volatile long widget = 0L;
private volatile long widget;
private long standaloneWindow;
private volatile boolean quit;

@@ -79,8 +79,8 @@ final class ListHelper implements XScrollbarClient {
// Holds the true if mouse is dragging outside of the area of the list
// The flag is used at the moment of the dragging and releasing mouse
// See 6243382 for more information
private boolean mouseDraggedOutVertically = false;
private volatile boolean vsbVisibilityChanged = false;
private boolean mouseDraggedOutVertically;
private volatile boolean vsbVisibilityChanged;

/*
* Comment
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -73,7 +73,7 @@ protected boolean createXIC() {
}


private static volatile long xicFocus = 0;
private static volatile long xicFocus;

protected void setXICFocus(ComponentPeer peer,
boolean value, boolean active) {
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -82,7 +82,7 @@ public void removeDropTarget(DropTarget dt) {
getLwTarget().removeDropTarget(dt);
}

private volatile long overriddenWindowHandle = 0L;
private volatile long overriddenWindowHandle;

@Override
public void overrideWindowHandle(final long handle) {
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -46,7 +46,7 @@ class XWarningWindow extends XWindow {
/**
* Animation stage.
*/
private volatile int currentIcon = 0;
private volatile int currentIcon;

/* -1 - uninitialized.
* 0 - 16x16
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2004, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -53,7 +53,7 @@ public final class ThemeReader {
new ReentrantReadWriteLock();
private static final Lock readLock = readWriteLock.readLock();
private static final Lock writeLock = readWriteLock.writeLock();
private static volatile boolean valid = false;
private static volatile boolean valid;
private static volatile boolean isThemed;

static volatile boolean xpStyleEnabled;
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1996, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1996, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -43,7 +43,7 @@ abstract class WObjectPeer {
private volatile boolean disposed;

// set from JNI if any errors in creating the peer occur
volatile Error createError = null;
volatile Error createError;

// used to synchronize the state of this peer
private final Object stateLock = new Object();