[
https://issues.apache.org/jira/browse/DERBY-5847?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Knut Anders Hatlen updated DERBY-5847:
--------------------------------------
Attachment: d5847-9a-this-leak.patch
d5847-8a-misc.patch
d5847-7a-sync-on-non-final.patch
d5847-6a-obsolete-collection.patch
Attaching four patches that address various warnings. I'll keep them as
separate patches so that it's easier to review the changes, or back out parts
of them later if some of the changes cause problems. The regression tests
passed.
Here's what the patches do:
* d5847-6a-obsolete-collection.patch:
Replace use of obsolete collection class Vector with ArrayList. No
synchronization is needed, as far as I can see, since the vectors are private
to a single server thread.
Also converted two of the original Vector fields (errorManagers and
errorManagersLevel) to local variables, as they are never used outside of the
method where they are created.
* d5847-7a-sync-on-non-final.patch:
NetBeans warns about synchronization on some non-final fields: timeSliceSync,
logConnectionsSync and closeSync, which protect the fields timeSlice,
logConnections and close, respectively.
The easiest fix would be to make the *Sync fields final. However, the
synchronization objects are only used in set/get methods to ensure that writes
in one thread are seen by reads in other threads. For this purpose, using
volatile fields would be sufficient, and the extra fields with synchronization
objects could be removed. The patch therefore declares timeSlice,
logConnections and close volatile, and it removes the timeSliceSync,
logConnectionsSync and closeSync fields.
* d5847-8a-misc.patch:
This patch fixes three warnings:
- An if statement had been disabled by prepending "false &&" to the condition,
so it never evaluated to true. A warning was shown because the code in the body
of the if statement was unreachable. The patch comments out the code, and also
adds a comment about why the code was disabled in the first place.
- An unnecessary bit shift operation was removed (shift bits 0 positions to the
left is a no-op).
- A local variable in setDatabase() hid a field and was renamed from rdbnam to
dbname.
* d5847-9a-this-leak.patch:
The warning that it addresses is about the constructor leaking a reference to
"this". That is, the constructor calls a method and passes "this" as an
argument. The problem is that the method to which "this" is passed sees a not
yet fully initialized instance. This may or may not be a problem depending on
which parts of the state the method needs to access, but it's best avoided.
The patch changes the name and signature of the method that receives the
reference to "this", from NetworkServerControlImpl.setUniqueThreadName(Thread
thrd, String newName) to NetworkServerControlImpl.getUniqueThreadName(String
base). The new method returns a unique thread name instead of setting the name
of thread. The constructor could therefore pass the returned value to the super
constructor instead of passing a reference to itself to the
setUniqueThreadName() method. In addition to silencing the warning, this
approach has the advantages that it's less code and the thread name doesn't
have to be set twice per thread.
A similar change had to be made to ClientThread's constructor, since
NetworkServerControlImpl.setUniqueThreadName() was also used there.
> Clean up IDE warnings in DRDAConnThread
> ---------------------------------------
>
> Key: DERBY-5847
> URL: https://issues.apache.org/jira/browse/DERBY-5847
> Project: Derby
> Issue Type: Improvement
> Components: Network Server
> Affects Versions: 10.10.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: d5847-1a-string-equality.patch,
> d5847-2a-unnecessary-return.patch, d5847-3a-static-fields-and-imports.patch,
> d5847-4a-unused-assignment.patch, d5847-5a-performance-warnings.patch,
> d5847-6a-obsolete-collection.patch, d5847-7a-sync-on-non-final.patch,
> d5847-8a-misc.patch, d5847-9a-this-leak.patch
>
>
> When I open DRDAConnThread in NetBeans, I see 49 warnings. Most of them are
> harmless (like static fields accessed via an instance, suggestions about
> using StringBuilder instead of StringBuffer, or using System.arraycopy()
> instead of for loops). Others indicate real problems, like the use of != to
> compare SQL states in writeSQLDIAGGRP().
> We should clean up the warnings so that it's easier to notice new warnings
> about potential problems.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
