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

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

Index: native_client_sdk/src/libraries/nacl_io/socket/udp_node.cc
diff --git a/native_client_sdk/src/libraries/nacl_io/socket/udp_node.cc b/native_client_sdk/src/libraries/nacl_io/socket/udp_node.cc
index 9b0bfc8f1801f9fa26c1c36f4259fb158b30fa96..7caef65640705e66dab0b7c9b5e2662987008c1b 100644
--- a/native_client_sdk/src/libraries/nacl_io/socket/udp_node.cc
+++ b/native_client_sdk/src/libraries/nacl_io/socket/udp_node.cc
@@ -134,6 +134,7 @@ class UdpRecvWork : public UdpWork {
     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();