[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: RFR: 8292214: Memory leak in getAllConfigs of awt_GraphicsEnv.c:386 [v2]
From:       Phil Race <prr () openjdk ! org>
Date:       2022-09-30 21:55:30
Message-ID: TPdgsdJFhNpt4PRz8lMSXPdorxy5v-shd49IzMA-SA8=.ed3f73c2-ea6b-4c2f-8789-6020bf10b8ed () github ! com
[Download RAW message or body]

On Thu, 22 Sep 2022 04:22:34 GMT, Prasanta Sadhukhan <psadhukhan@openjdk.org> wrote:

> > Alisen Chung has updated the pull request incrementally with one additional \
> > commit since the last revision: 
> > changed loop end condition to ind-1
> 
> src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c line 578:
> 
> > 576:             if (graphicsConfigs[i] != 0) {
> > 577:                 free(graphicsConfigs[i]);
> > 578:             }
> 
> It seems  `screenDataPtr->numConfigs` is updated only when `success = JNI_TRUE` and \
> here we are checking if `success != JNI_TRUE` condition, so amnot sure if we will \
> be getting the correct updated configs number. I think condition should be `i< \
> ind-1` which is updated before every allocation, but it should be verified.

I think "nConfig" would have been the better value to use.
Look at how the array is allocated and that it is zeroed out
   nConfig = n8p + n12p + n8s + n8gs + n8sg  + n1sg + nTrue + 1;
    graphicsConfigs = (AwtGraphicsConfigDataPtr *)
        calloc(nConfig, sizeof(AwtGraphicsConfigDataPtr));

So it should always be safe.

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

PR: https://git.openjdk.org/jdk/pull/10378


[prev in list] [next in list] [prev in thread] [next in thread]