From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bernhard Bitsch To: development@lists.ipfire.org Subject: Aw: Re: [PATCH 1/5] location-functions.pl: Use a single script-wide db_handle. Date: Mon, 09 Nov 2020 13:13:09 +0100 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1032260398782017309==" List-Id: --===============1032260398782017309== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, > Gesendet: Montag, 09. November 2020 um 12:35 Uhr > Von: "Michael Tremer" > An: "Bernhard Bitsch" > Cc: development(a)lists.ipfire.org > Betreff: Re: [PATCH 1/5] location-functions.pl: Use a single script-wide db= _handle. > > Hello, > > Thank you Stefan for submitting this patchset. > > > On 8 Nov 2020, at 19:36, Bernhard Bitsch wrote: > > > > This means, we stay with the unbalanced memory allocation in (Perl) liblo= c. Which leaves a memory leak. > > Bernhard, could you please elaborate on how this memory leak is still exist= ing? > > I also do not understand what you mean by unbalanced. > > As far as I understand the code right now, the database is being opened onc= e and the handle is being stored internally. All functions that are being cal= led will no longer have to hold their own database handle. Therefore the maxi= mum amount of handles open is one. > The init() function allocates the handle, which is not really destroyed. The = END block is just a safety process. The allocated memory for the handle shoul= d be released in the moment of references=3D=3D0. Yes, the new code ( inspired by my work-around ) minimizes the number of hand= les to one. But this handle is persistent also ( this doesn't hurt because it= is used persistent for the time of module use ). > > > >> Gesendet: Samstag, 07. November 2020 um 19:47 Uhr > >> Von: "Stefan Schantl" > >> An: development(a)lists.ipfire.org > >> Betreff: [PATCH 1/5] location-functions.pl: Use a single script-wide db_= handle. > >> > >> Create and use a single script-wide database handle for libloc to > >> prevent from creating multiple ones. > >> > >> This helps saving memory, especially on small systems. > >> > >> Reference #12515. > >> > > > > The error can be produced easily with small memory, but it is present in = all systems. > > Therefore I've posted this solution as work-around only! > > Did you test this patchset or did you come to your conclusion by only readi= ng the code? > > Best, > -Michael > The sentence about "small systems" refers to the original comment. It is not = a real memory saving, the patch cures a 'memory wasting'. It is clear, that a= massively use of malloc without free crashes a small system faster than a sy= stem with big memory resources. The patchset is mainly my work-around, thus it is in test since I found the e= rror. Best, -Bernhard > > > > - Bernhard > > > > > >> Signed-off-by: Stefan Schantl > >> --- > >> config/cfgroot/location-functions.pl | 11 ++++------- > >> 1 file changed, 4 insertions(+), 7 deletions(-) > >> > >> diff --git a/config/cfgroot/location-functions.pl b/config/cfgroot/locat= ion-functions.pl > >> index 2cfe7f908..9b1d0bfb5 100644 > >> --- a/config/cfgroot/location-functions.pl > >> +++ b/config/cfgroot/location-functions.pl > >> @@ -55,6 +55,9 @@ our $keyfile =3D "$location_dir/signing-key.pem"; > >> # Directory which contains the exported databases. > >> our $xt_geoip_db_directory =3D "/usr/share/xt_geoip/"; > >> > >> +# Create libloc database handle. > >> +my $db_handle =3D &init(); > >> + > >> # > >> ## Tiny function to init the location database. > >> # > >> @@ -86,7 +89,7 @@ sub verify ($) { > >> ## Function to the the country code of a given address. > >> # > >> sub lookup_country_code($$) { > >> - my ($db_handle, $address) =3D @_; > >> + my ($address) =3D @_; > >> > >> # Lookup the given address. > >> my $country_code =3D &Location::lookup_country_code($db_handle, $addres= s); > >> @@ -174,9 +177,6 @@ sub get_full_country_name($) { > >> > >> # Function to get all available locations. > >> sub get_locations() { > >> - # Create libloc database handle. > >> - my $db_handle =3D &init(); > >> - > >> # Get locations which are stored in the location database. > >> my @database_locations =3D &Location::database_countries($db_handle); > >> > >> @@ -197,9 +197,6 @@ sub address_has_flags($) { > >> # Array to store the flags of the address. > >> my @flags; > >> > >> - # Init libloc database handle. > >> - my $db_handle =3D &init(); > >> - > >> # Loop through the hash of possible network flags. > >> foreach my $flag (keys(%network_flags)) { > >> # Check if the address has the current flag. > >> -- > >> 2.20.1 > >> > >> > > --===============1032260398782017309==--