[
https://issues.apache.org/jira/browse/ZOOKEEPER-5013?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ConfX updated ZOOKEEPER-5013:
-----------------------------
Description:
The `QuorumPeerTestBase.MainThread` test utility class has inconsistent
constructor implementations. One constructor chain (lines 203-260) does not
initialize the `clientPort`, `myid`, `quorumCfgSection`, and `otherConfigs`
member variables, causing `getClientPort()` and `getMyid()` to return incorrect
default values (0).
h3. Constructor at lines 116-161 - CORRECT
This constructor properly initializes all member variables:
{code:java}
public MainThread(int myid, int clientPort, String quorumCfgSection,
Map<String, String> otherConfigs, int tickTime) throws
IOException {
baseDir = ClientBase.createTmpDir();
this.myid = myid;
this.clientPort = clientPort; // ✓ CORRECTLY INITIALIZED
this.quorumCfgSection = quorumCfgSection; // ✓ CORRECTLY INITIALIZED
this.otherConfigs = otherConfigs; // ✓ CORRECTLY INITIALIZED
LOG.info("id = {} tmpDir = {} clientPort = {}", myid, baseDir, clientPort);
confFile = new File(baseDir, "zoo.cfg");
FileWriter fwriter = new FileWriter(confFile);
fwriter.write("tickTime=" + tickTime + "\n");
// ... writes config file ...
fwriter.write("clientPort=" + clientPort + "\n");
// ... rest of constructor ...
} {code}
h3. Constructor at lines 203-260 - BUGGY
{code:java}
public MainThread(int myid, int clientPort, int adminServerPort,
Integer secureClientPort, String quorumCfgSection,
String configs, String peerType,
boolean writeDynamicConfigFile, String version) throws
IOException {
tmpDir = ClientBase.createTmpDir();
// ✗ MISSING: this.myid = myid;
// ✗ MISSING: this.clientPort = clientPort;
// ✗ MISSING: this.quorumCfgSection = quorumCfgSection;
// ✗ MISSING: this.otherConfigs = ...;
LOG.info("id = {} tmpDir = {} clientPort = {} adminServerPort = {}",
myid, tmpDir, clientPort, adminServerPort); // Logs it but
doesn't store!
File dataDir = new File(tmpDir, "data");
// ... writes config file ...
if (clientPort != UNSET_STATIC_CLIENTPORT) {
fwriter.write("clientPort=" + clientPort + "\n"); // Writes to file
but not to member!
}
// ... rest of constructor - member variables never assigned ...
} {code}
When tests use a constructor that eventually calls the buggy constructor chain
(lines 175 → 195 → 199 → 203), the following methods return incorrect values:
- `getClientPort()` returns 0 instead of the actual port
- `getMyid()` returns 0 instead of the actual myid
- `getQuorumCfgSection()` returns null
- `getOtherConfigs()` returns null
h2. Fix
{code:java}
public MainThread(int myid, int clientPort, int adminServerPort,
Integer secureClientPort, String quorumCfgSection,
String configs, String peerType,
boolean writeDynamicConfigFile, String version) throws
IOException {
tmpDir = ClientBase.createTmpDir();
// ADD THESE LINES:
this.myid = myid;
this.clientPort = clientPort;
this.quorumCfgSection = quorumCfgSection;
this.otherConfigs = null; // This constructor doesn't have otherConfigs
parameter
LOG.info("id = {} tmpDir = {} clientPort = {} adminServerPort = {}",
myid, tmpDir, clientPort, adminServerPort);
// ... rest of constructor unchanged ...
} {code}
I'm happy to send a PR for this issue.
was:
The `QuorumPeerTestBase.MainThread` test utility class has inconsistent
constructor implementations. One constructor chain (lines 203-260) does not
initialize the `clientPort`, `myid`, `quorumCfgSection`, and `otherConfigs`
member variables, causing `getClientPort()` and `getMyid()` to return incorrect
default values (0).
h3. Constructor at lines 116-161 - CORRECT
This constructor properly initializes all member variables:
{code:java}
public MainThread(int myid, int clientPort, String quorumCfgSection,
Map<String, String> otherConfigs, int tickTime) throws
IOException {
baseDir = ClientBase.createTmpDir();
this.myid = myid;
this.clientPort = clientPort; // ✓ CORRECTLY INITIALIZED
this.quorumCfgSection = quorumCfgSection; // ✓ CORRECTLY INITIALIZED
this.otherConfigs = otherConfigs; // ✓ CORRECTLY INITIALIZED
LOG.info("id = {} tmpDir = {} clientPort = {}", myid, baseDir, clientPort);
confFile = new File(baseDir, "zoo.cfg");
FileWriter fwriter = new FileWriter(confFile);
fwriter.write("tickTime=" + tickTime + "\n");
// ... writes config file ...
fwriter.write("clientPort=" + clientPort + "\n");
// ... rest of constructor ...
} {code}
h3. Constructor at lines 203-260 - BUGGY
{code:java}
public MainThread(int myid, int clientPort, int adminServerPort,
Integer secureClientPort, String quorumCfgSection,
String configs, String peerType,
boolean writeDynamicConfigFile, String version) throws
IOException {
tmpDir = ClientBase.createTmpDir();
// ✗ MISSING: this.myid = myid;
// ✗ MISSING: this.clientPort = clientPort;
// ✗ MISSING: this.quorumCfgSection = quorumCfgSection;
// ✗ MISSING: this.otherConfigs = ...;
LOG.info("id = {} tmpDir = {} clientPort = {} adminServerPort = {}",
myid, tmpDir, clientPort, adminServerPort); // Logs it but
doesn't store!
File dataDir = new File(tmpDir, "data");
// ... writes config file ...
if (clientPort != UNSET_STATIC_CLIENTPORT) {
fwriter.write("clientPort=" + clientPort + "\n"); // Writes to file
but not to member!
}
// ... rest of constructor - member variables never assigned ...
} {code}
When tests use a constructor that eventually calls the buggy constructor chain
(lines 175 → 195 → 199 → 203), the following methods return incorrect values:
- `getClientPort()` returns 0 instead of the actual port
- `getMyid()` returns 0 instead of the actual myid
- `getQuorumCfgSection()` returns null
- `getOtherConfigs()` returns null
h2. Fix
{code:java}
public MainThread(int myid, int clientPort, int adminServerPort,
Integer secureClientPort, String quorumCfgSection,
String configs, String peerType,
boolean writeDynamicConfigFile, String version) throws
IOException {
tmpDir = ClientBase.createTmpDir(); // ADD THESE LINES:
this.myid = myid;
this.clientPort = clientPort;
this.quorumCfgSection = quorumCfgSection;
this.otherConfigs = null; // This constructor doesn't have otherConfigs
parameter
LOG.info("id = {} tmpDir = {} clientPort = {} adminServerPort = {}",
myid, tmpDir, clientPort, adminServerPort);
// ... rest of constructor unchanged ...
} {code}
I'm happy to send a PR for this issue.
> QuorumPeerTestBase.MainThread constructor chain does not initialize member
> variables
> ------------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-5013
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-5013
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
> Reporter: ConfX
> Priority: Major
>
> The `QuorumPeerTestBase.MainThread` test utility class has inconsistent
> constructor implementations. One constructor chain (lines 203-260) does not
> initialize the `clientPort`, `myid`, `quorumCfgSection`, and `otherConfigs`
> member variables, causing `getClientPort()` and `getMyid()` to return
> incorrect default values (0).
>
> h3. Constructor at lines 116-161 - CORRECT
> This constructor properly initializes all member variables:
> {code:java}
> public MainThread(int myid, int clientPort, String quorumCfgSection,
> Map<String, String> otherConfigs, int tickTime) throws
> IOException {
> baseDir = ClientBase.createTmpDir();
> this.myid = myid;
> this.clientPort = clientPort; // ✓ CORRECTLY INITIALIZED
> this.quorumCfgSection = quorumCfgSection; // ✓ CORRECTLY INITIALIZED
> this.otherConfigs = otherConfigs; // ✓ CORRECTLY INITIALIZED
> LOG.info("id = {} tmpDir = {} clientPort = {}", myid, baseDir,
> clientPort);
> confFile = new File(baseDir, "zoo.cfg");
> FileWriter fwriter = new FileWriter(confFile);
> fwriter.write("tickTime=" + tickTime + "\n");
> // ... writes config file ...
> fwriter.write("clientPort=" + clientPort + "\n");
> // ... rest of constructor ...
> } {code}
> h3. Constructor at lines 203-260 - BUGGY
> {code:java}
> public MainThread(int myid, int clientPort, int adminServerPort,
> Integer secureClientPort, String quorumCfgSection,
> String configs, String peerType,
> boolean writeDynamicConfigFile, String version) throws
> IOException {
> tmpDir = ClientBase.createTmpDir();
> // ✗ MISSING: this.myid = myid;
> // ✗ MISSING: this.clientPort = clientPort;
> // ✗ MISSING: this.quorumCfgSection = quorumCfgSection;
> // ✗ MISSING: this.otherConfigs = ...;
> LOG.info("id = {} tmpDir = {} clientPort = {} adminServerPort = {}",
> myid, tmpDir, clientPort, adminServerPort); // Logs it but
> doesn't store!
> File dataDir = new File(tmpDir, "data");
> // ... writes config file ...
> if (clientPort != UNSET_STATIC_CLIENTPORT) {
> fwriter.write("clientPort=" + clientPort + "\n"); // Writes to file
> but not to member!
> }
> // ... rest of constructor - member variables never assigned ...
> } {code}
> When tests use a constructor that eventually calls the buggy constructor
> chain (lines 175 → 195 → 199 → 203), the following methods return incorrect
> values:
> - `getClientPort()` returns 0 instead of the actual port
> - `getMyid()` returns 0 instead of the actual myid
> - `getQuorumCfgSection()` returns null
> - `getOtherConfigs()` returns null
> h2. Fix
> {code:java}
> public MainThread(int myid, int clientPort, int adminServerPort,
> Integer secureClientPort, String quorumCfgSection,
> String configs, String peerType,
> boolean writeDynamicConfigFile, String version) throws
> IOException {
> tmpDir = ClientBase.createTmpDir();
> // ADD THESE LINES:
> this.myid = myid;
> this.clientPort = clientPort;
> this.quorumCfgSection = quorumCfgSection;
> this.otherConfigs = null; // This constructor doesn't have otherConfigs
> parameter
>
> LOG.info("id = {} tmpDir = {} clientPort = {} adminServerPort = {}",
> myid, tmpDir, clientPort, adminServerPort);
> // ... rest of constructor unchanged ...
> } {code}
> I'm happy to send a PR for this issue.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
