Description
In Proposer.java, the instanceState map is a plain in-memory HashMap that is not persisted to disk. If a Proposer crashes and restarts, all instance state is lost — the map is empty. However, Accepters may still send back delayed PrepareResponsePacket or AcceptResponsePacket for instances that existed before the crash. When the restarted Proposer receives these responses, this.instanceState.get(instance) returns null, causing a NullPointerException.
How to Trigger
- Proposer starts a Paxos round for instance i, broadcasts Prepare/Accept to Accepters.
- Proposer crashes and restarts. instanceState is now an empty HashMap.
- Accepters that received the Proposer's messages before the crash eventually reply with PrepareResponsePacket or AcceptResponsePacket referencing instance i.
- The restarted Proposer calls onPrepareResponse() or onAcceptResponce() . this.instanceState.get(instance) returns null.
- Subsequent field accesses like current.state throw NullPointerException.
Why This Is a Code Smell
Although the NullPointerException is silently caught by the catch(Exception e) block in the message loop, and the loop continues — effectively behaving the same as a return — using exception handling as a control flow mechanism is a well-known code smell. In particular, it causes the intent unclear: The code does not express the intent that "responses for unknown instances should be ignored." A reader must infer this from the exception handling, which is non-obvious.
Suggested Fix
Quick fix — add explicit null checks in onPrepareResponse() and onAcceptResponce(), and return early if the instance is not found:
// In onPrepareResponse():
Instance current = this.instanceState.get(instance);
if (current == null) {
return;
}
// In onAcceptResponce():
Instance current = this.instanceState.get(instance);
if (current == null) {
return;
}
This makes the intent explicit: responses for instances that this Proposer does not currently track should simply be discarded, and eliminates the exception-based control flow.
Description
In Proposer.java, the instanceState map is a plain in-memory HashMap that is not persisted to disk. If a Proposer crashes and restarts, all instance state is lost — the map is empty. However, Accepters may still send back delayed PrepareResponsePacket or AcceptResponsePacket for instances that existed before the crash. When the restarted Proposer receives these responses, this.instanceState.get(instance) returns null, causing a NullPointerException.
How to Trigger
Why This Is a Code Smell
Although the NullPointerException is silently caught by the catch(Exception e) block in the message loop, and the loop continues — effectively behaving the same as a return — using exception handling as a control flow mechanism is a well-known code smell. In particular, it causes the intent unclear: The code does not express the intent that "responses for unknown instances should be ignored." A reader must infer this from the exception handling, which is non-obvious.
Suggested Fix
Quick fix — add explicit null checks in onPrepareResponse() and onAcceptResponce(), and return early if the instance is not found:
This makes the intent explicit: responses for instances that this Proposer does not currently track should simply be discarded, and eliminates the exception-based control flow.