On Tue, 16 Feb 2021 12:26:54 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:

> In another bug  this question from me  was answered by  Alan Bateman :
> 
> Btw. while adjusting Java_sun_nio_ch_sctp_SctpChannelImpl_receive0 , I 
> started to wonder what happens to the allocated memory in the same file in 
> handleSendFailed ( if ((addressP = malloc(dataLength)) == NULL) ) in early 
> return cases incl. the CHECK_NULL , is there some deallocation missing there 
> too ?
> 
> 
> Yes, the error paths in handleSendFailed should be looked at. If 
> NewDirectByteBuffer or recvmsg fails then addressP needs to be freed. 
> Furthermore, if the NewObject fails and bufferObj != NULL then the memory for 
> the direct buffer will need to be freed too (as JNI NewDirectByteBuffer does 
> not setup a cleaner).
> 
> 
> So I added freeing of the malloced memory to handleSendFailed .
> Please review !
> 
> Thanks, Matthias

Marked as reviewed by chegar (Reviewer).

src/jdk.sctp/unix/native/libsctp/SctpChannelImpl.c line 236:

> 234:             return;
> 235:         }
> 236: 

This looks fine. If NewDirectByteBuffer returns NULL there will be a pending 
OutOfMemoryError.

src/jdk.sctp/unix/native/libsctp/SctpChannelImpl.c line 251:

> 249:                 free(addressP);
> 250:                 handleSocketError(env, errno);
> 251:                 return;

At this point the direct ByteBuffer has been successfully allocated and refers 
to the memory at addressP. But the direct ByteBuffer, while in the Java heap, 
will not have any references to it, so freeing addressP should be fine here.

src/jdk.sctp/unix/native/libsctp/SctpChannelImpl.c line 257:

> 255:                 //TODO: assert false: "should not reach here";
> 256:                 free(addressP);
> 257:                 return;

same a previous comment - looks ok.

src/jdk.sctp/unix/native/libsctp/SctpChannelImpl.c line 270:

> 268:         return;
> 269:     }
> 270:     (*env)->SetObjectField(env, resultContainerObj, src_valueID, 
> resultObj);

If NewObject returns NULL, there will be a pending OutOfMemoryError. Freeing 
addressP should be safe here, same reasoning as above.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2586

Reply via email to