db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Knut Anders Hatlen (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (DERBY-5847) Clean up IDE warnings in DRDAConnThread
Date Tue, 10 Jul 2012 14:15:34 GMT

     [ 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

        

Mime
View raw message