`NullPointerException` at `transferDone()` in `DecisionEngineRouter.java`

[oleotiger]

In DecisionEngineRouter.java ,
Message transferred = this.getMessage(con.getMessage().getId()); may get transferred=null as con.getMessage() will return a message which is not in messages in this router already. getMessage() will return null on this condition.

This may occur when eg. M25 is transferred to N12 while the destination of M25 is N16 and N16 has received the message. Tuple <M25,N1>, <M25,N16>, <M25, N2> are added to outgoingMessages. In startTransfer() when test <M25,N16> , M25 should be deleted on N12. But tuple <M25,N2> is still in outgoingMessages and startTransfer() will return rcv_ok.

What's more, when a message is delete for ttl but it's still on transferring, what will happen? I think the transmission should be aborted but I didn't find where the program handle this situation

[denatajp]

So, I was helping my friend with their thesis project when I came across this error in DecisionEngineRouter.java

I ran into a NullPointerException at line 411, and after looking into it, I realized the issue happens because transferred can sometimes be null. This occurs when con.getMessage() returns a message that is no longer in the router’s list, which causes getMessage() to return null.

After analyzing the situation, I found that this can happen when a message, say M25, is being transferred between nodes. For example, M25 is being sent from N12 to N16, and at the same time, N16 already received and deleted it. However, there are still references to M25 in outgoingMessages, such as tuples like <M25, N1>, <M25, N16>, and <M25, N2>.

When the transfer process checks the tuple <M25, N16>, it realizes that M25 should be removed from N12. But the tuple <M25, N2> is still there, and the transfer process continues as if M25 still exists in the system. This leads to a situation where getMessage() returns null, causing the crash.

Another possible scenario is when a message is deleted due to TTL expiration, but it is still being transferred. In this case, the transmission should ideally be aborted, but there wasn’t any handling for this situation in the original code.

To fix this, I made the following changes:

@Override
protected void transferDone(Connection con) {
  Message msgTransferred = con.getMessage();
  if (msgTransferred == null) {
      return;
  }
  String msgId = msgTransferred.getId();
  
  // Remove tuple from outgoingMessages for this message and connection
  for (Iterator<Tuple<Message, Connection>> i = outgoingMessages.iterator(); i.hasNext();) {
      Tuple<Message, Connection> t = i.next();
      if (t.getKey().getId().equals(msgId) && t.getValue().equals(con)) {
          i.remove();
          break;
      }
  }
  
  // Check if the message is still in the router
  Message transferred = this.getMessage(msgId);
  if (transferred != null && decider.shouldDeleteSentMessage(transferred, con.getOtherNode(getHost()))) {
      // Delete the message and all related tuples
      this.deleteMessage(msgId, false);
      for (Iterator<Tuple<Message, Connection>> i = outgoingMessages.iterator(); i.hasNext();) {
          Tuple<Message, Connection> t = i.next();
          if (t.getKey().getId().equals(msgId)) {
              i.remove();
          }
      }
  }
}

First, I added a check so that if con.getMessage() returns null, the function exits immediately, preventing the NullPointerException. Then, I ensured that whenever a message is deleted, all related tuples in outgoingMessages are also removed. This prevents leftover references from causing unexpected behavior.

Sorry if my English is not perfect, English is not my first language