[NaCl SDK] nacl_io: Fix leak when recv'ing from udp socket.

native_client_sdk/src/libraries/nacl_io/socket/udp_node.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "nacl_io/socket/udp_node.h"
 
 #include <errno.h>
 #include <string.h>
 
 #include <algorithm>
@@ skipping 116 matching lines @@
   virtual void Run(int32_t length_error) {
     AUTO_LOCK(emitter_->GetLock());
     UdpNode* stream = static_cast<UdpNode*>(emitter_->stream());
     if (NULL == stream)
       return;
 
     // On successful receive we queue more input
     if (length_error > 0) {
       Packet* packet = new Packet(filesystem()->ppapi());
       packet->Copy(data_, length_error, addr_);
+      filesystem()->ppapi()->ReleaseResource(addr_);

[yzshen1, 2014/08/19 18:37:29]

drive-by:
Maybe we could consider "safer" ref-counting.
- addr_ is not reset but this class doesn't own a ref after this
ReleaseResource() call.
- if Start() is called but Run() isn't, then addr_ is eventually leaked.

(I don't know much about this part of code base, though. Just my 2 cents. :))

[noelallen1, 2014/08/19 20:46:59]

While it is possible to call Start without calling Run, it shouldn't be possible
RecvFrom to populate addr_ without also calling the callback correct?


On 2014/08/19 18:37:29, yzshen1 wrote:

[yzshen1, 2014/08/19 20:53:16]

Thanks for reply!

If the completion callback is "required", then it will be run for sure. I just
searched the code for GetRunCompletion(). And you are right, it is a "required"
callback.

(Optional callback could provide better performance in some cases. Pepper can
choose to complete the call synchronously and skip the callback.)

       emitter_->WriteRXPacket_Locked(packet);
       stream->ClearStreamFlags(SSF_RECVING);
       stream->QueueInput();
     } else {
       stream->SetError_Locked(length_error);
     }
   }
 
  private:
   char data_[kMaxPacketSize];
@@ skipping 148 matching lines @@
   int capped_len = static_cast<int32_t>(std::min<int>(len, kMaxPacketSize));
   Packet* packet = new Packet(filesystem_->ppapi());
   packet->Copy(buf, capped_len, addr);
 
   emitter_->WriteTXPacket_Locked(packet);
   *out_len = capped_len;
   return 0;
 }
 
 }  // namespace nacl_io