IBM / db2sock-ibmi

An asynchronous PASE Db2 and IBM i integration library
MIT License
4 stars 7 forks source link

SQL400Json - memory leak ? #22

Closed kadler closed 6 years ago

kadler commented 6 years ago

Original report by Sebastian Misiewicz (Bitbucket: SMisiewicz, GitHub: sebastian-misiewicz).


Hi

I have created a simple program which calls SQL400Json in a loop.

#!c

#include <stdio.h>
#include <sqlcli1.h>
#include "PaseCliAsync.h"

  char injson[] = "{\"script\":[ {\"cmd\":{\"exec\":\"CHGLIBL LIBL(MYLIB QTEMP) CURLIB(MYLIB)\"}},\
  {\"pgm\":[{\"name\":\"HELLO\"},\
  {\"s\":[{\"name\":\"char\", \"type\":\"10000000a\", \"value\":{\"Hi there\"}},\
          {\"name\":\"char\", \"type\":\"128a\", \"value\":\"Hi there\"}]} ]}]}\x00";

char outjson[1000000];

void calljson()
{
  SQLINTEGER inlen = strlen(injson);
  outjson[0] = 0;
  SQLINTEGER outlen = sizeof(outjson);
  SQL400Json(0, injson, inlen, outjson, outlen);
  printf("result %s\n", outjson);
}

int main()
{
  for(int i = 0; i < 100; i++) {
   calljson();
   sleep(5);
  }
  return 0;
}

Looks like it doesn't free memory or did I miss something obvious?

Regards

Sebastian

kadler commented 6 years ago

Original comment by Sebastian Misiewicz (Bitbucket: SMisiewicz, GitHub: sebastian-misiewicz).


All good now.

kadler commented 6 years ago

Original comment by Teemu Halmela (Bitbucket: teemu_, GitHub: Unknown).


I have also couple of more fixes and I really should make issues for those so they get properly integrated.

kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


Halmela thanks

Thanks for helping. An obviously talented c programmer. I see your fork up to 1.1.3-sg6. My bad. I typed before looking at your ;latest fork version in my deleted post. Sorry, too fast for my own good some days (old guy ... no excuse ... but true).

Anyway, I see you are looking at timings in fork. Great! Please help our performance conscience. Thanks again.

kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


leak ...

Ok, new version posted to fix leak. This driver removes all dependencies on old PASE libdb400.a.

Yips Super Driver - test driver - 1.1.4-sg1 - toolkit - remove old PASE libdb400.a dependecies (also fix leak json call)

Thanks for test help on db2sock open source project.

kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


leak ..

Ok, appears stabilized heap (no leak). Thanks for the find. I will post soon.

#!bash

bash-4.3$ ./test1010_jsonloop32   
input(5000000):
{"pgm":[{"name":"HELLOSRV", "lib":"DB2JSON", "func":"HELLO"},{"s":{"name":"char", "type":"128a", "value":"Hi there"}}]}

output:
lowwater(20520e80) == highwater(20520e80) - loop count (0)
lowwater(20520e80) == highwater(20520e80) - loop count (100)
lowwater(20520e80) == highwater(20520e80) - loop count (200)
lowwater(20520e80) == highwater(20520e80) - loop count (300)
lowwater(20520e80) == highwater(20520e80) - loop count (400)
lowwater(20520e80) == highwater(20520e80) - loop count last (500)
success (0)
bash-4.3$ ./test1010_jsonloop64
input(5000000):
{"pgm":[{"name":"HELLOSRV", "lib":"DB2JSON", "func":"HELLO"},{"s":{"name":"char", "type":"128a", "value":"Hi there"}}]}

output:
lowwater(1805210c0) == highwater(1805210c0) - loop count (0)
lowwater(1805210c0) == highwater(1805210c0) - loop count (100)
lowwater(1805210c0) == highwater(1805210c0) - loop count (200)
lowwater(1805210c0) == highwater(1805210c0) - loop count (300)
lowwater(1805210c0) == highwater(1805210c0) - loop count (400)
lowwater(1805210c0) == highwater(1805210c0) - loop count last (500)
success (0)
bash-4.3$ 

FYI easier heap looking ...

FYI - sbrk(0) returns current heap break line. A good way to see if heap is growing without all the effort you have been using.

#!c

  /* json call (hdbc=0 - json handles connection) */
  for (i = 0, sqlrc = SQL_SUCCESS; i < 500 && sqlrc == SQL_SUCCESS; i++) {
    memset(outjson, 0, outlen);
    sqlrc = SQL400Json(0, injson, inlen, outjson, outlen);
    highwater = sbrk(0);
    if (!lowwater) {
      lowwater = highwater;
    }
    if (!(i % 100)) {
      printf("lowwater(%p) == highwater(%p) - loop count (%d)\n", lowwater, highwater, i);
    }
  }

MALLOCDEBUG - AIX/PASE built-in flight heap recorder

A easy way to see heap allocations not free is using AIX/PASE MALLOCDEBUG=report_allocations. However, report is 'enthusiastic', wherein not all leaked allocations are equally important.

#!bash

bash-4.3$ export MALLOCDEBUG=report_allocations

bash-4.3$ ./test1010_jsonloop32                
input(5000000):
{"pgm":[{"name":"HELLOSRV", "lib":"DB2JSON", "func":"HELLO"},{"s":{"name":"char", "type":"128a", "value":"Hi there"}}]}

output:
lowwater(2052ce80) == highwater(2052ce80) - loop count (0)

:

Current allocation report:

    Allocation #0: 0x2000CE30
        Allocation size: 0x2A0
        Allocated from heap: 0
        Allocation traceback:
        0xDF8F8194  malloc
        0xDF847E2C  init_malloc
        0xDF8498B4  malloc
        0xDFD97F14  __pth_init
kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


I think its because of the tpgm->layout memory isn't freed.

Thanks guys. Indeed, a leak in db2sock boat. I will add to master branch next commit.

BTW - Sebastian -- much better explain in follow up. I thought you asked for a code review of your toy program.

#!c
SQLRETURN tool_key_pgm_run(tool_struct_t * tool, tool_node_t ** curr_node) {

  /* memory leak - delete used pgm template memeory (Misiewicz/Halmela) */
  if(tpgm->layout){
    tool_free(tpgm->layout);
  }

}

BTW -- Bump ahead ... next commit we remove dependency old PASE libdb400.a. Cross your fingers, buckle your seat belt, db2sock may hit some turbulence. We really need this thing to stand on it's own (completely).

kadler commented 6 years ago

Original comment by Sebastian Misiewicz (Bitbucket: SMisiewicz, GitHub: sebastian-misiewicz).


Indeed that did the trick. Thanks a lot

kadler commented 6 years ago

Original comment by Teemu Halmela (Bitbucket: teemu_, GitHub: Unknown).


I've also seen this memory leak happen and I think its because of the tpgm->layout memory isn't freed.

https://bitbucket.org/teemu_/db2sock/commits/2c296073679a3ff3ed51fd05ca08beabee53fb7f

kadler commented 6 years ago

Original comment by Sebastian Misiewicz (Bitbucket: SMisiewicz, GitHub: sebastian-misiewicz).


Hi

Thanks for quick answer. Yes I'm a casual C programmer and it's a kind of a toy.

Let's back to that tiny program and leave as is now (even wih global / static outjson and outjson[0] = 0 instead of memset memset(outjson......)

compile it

#!bash

gcc  -I..     -c json400.c -o json400.o
gcc -L.. -ldb400 json400.o -o json400

and run it (I use screen command to send it to the background kind of SBMJOB ),

switch to iSeries GoB and look for that job with WRKACTJOB then

#!bash

3.  Display job run attributes, if active 

look at

#!bash

 Maximum temporary storage in megabytes  . . . . . :   *NOMAX
   Temporary storage used  . . . . . . . . . . . . :     164 
   Peak temporary storage used . . . . . . . . . . :     164 

refresh it after a minute or two

#!bash

  Temporary storage used  . . . . . . . . . . . . :     260   
  Peak temporary storage used . . . . . . . . . . :     260   

or use SQL

#!sql

SELECT temporary_storage, function, x.*
FROM TABLE (QSYS2.ACTIVE_JOB_INFO()) AS X where JOB_NAME like  '%QSECOFR%'  and function='json400';

if you wait more time most likely the program will be killed (leaving core file)

That's odd there is no alloc/free there.

Decrease the size of the array size from "type":"10000000a" to "type":"1000000a" then start all over again

The same, program crashes but in this case it might take some more time.

Last test, comment out

#!c

//SQL400Json(0, injson, inlen, outjson, outlen);

Compile and start it again.

Look at

#!bash

 Temporary storage used  . . . . . . . . . . . . :     6     
 Peak temporary storage used . . . . . . . . . . :     6     

and it remains on that level all the time - that's ok there is no alloc/free.

There is something wrong here ....

Thank you for explanation of memory management/PASE layout it's very valuable to me.

Regards Sebastian

kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


Oh yeah, for those reading ... there was no memory leak in original example. So ... unless he was talking about something completely different than the sample program (imagine we will know later post).

kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


more unsolicited ...

Above we discussed 2 of 3 types of memory for outjson.

1) static/global outjson outside function calljson (your original) ... not thread safe without mutex. Needs memset between calls.

#!c

char outjson[1000000]; /* static variable (bss technical term) */

void calljson()
{
  SQLINTEGER inlen = strlen(injson);
  outjson[0] = 0;
  SQLINTEGER outlen = sizeof(outjson);
  memset(outjson,0,sizeof(outjson)); /* zero any previous call junk */
  SQL400Json(0, injson, inlen, outjson, outlen);
  printf("result %s\n", outjson);
}

2) stack outjson inside function calljson ... thread safe, but may blow stack size. Needs memset to init previous use of stack (by any other call).

#!c

void calljson()
{
  char outjson[1000000]; /* on stack (safe but may blow stack size) */
  SQLINTEGER inlen = strlen(injson);
  outjson[0] = 0;
  SQLINTEGER outlen = sizeof(outjson);
  memset(outjson,0,sizeof(outjson)); /* zero any previous stack junk */
  SQL400Json(0, injson, inlen, outjson, outlen);
  printf("result %s\n", outjson);
}

3) Let's l add number 3 memory ... heap (malloc/free). Thread safe because malloc/free are thread safe APIs. Needs memset to init previous use of heap (by any other malloc/free).

#!c

void calljson()
{
  char * outjson = (char *) malloc(1000000); /* heap */
  SQLINTEGER inlen = strlen(injson);
  outjson[0] = 0;
  SQLINTEGER outlen = sizeof(outjson);
  memset(outjson,0,sizeof(outjson)); /* zero any previous heap use junk */
  SQL400Json(0, injson, inlen, outjson, outlen);
  printf("result %s\n", outjson);
  free(outjson);
}

4) There is also shared memory (shmat, mmap), but these are usually used as IPC between process when 'named' (IFS name). Although you can create anonymous shared memory and do some really cool things like create on the fly code and transfer control (same as java jit).

what is in an address???

Essentially each type of memory is mapped to a different 'area' of the flat process scoped address space by kernel (loader, etc.), depending on 32 bit or 64 bit process.

32 bit ...

1) static/ global outjson (not shown - shared object, libmyc.a, mything.so, but not main program) - range 0xf0000000 to 0xffffffff. Wherein, your process creates a copy-on-write page from original zero's bss copy set by loader. The data storage is outside your function calljson.

2) stack outjson - range 0x2fffffff to 0x20000000. Wherein as your program runs stack is 'used' to fit your variables inside your function call json. Also static main program global outjson allocates chunk this memory (lower toward 0x20000000). Basically 0x2 segment is a garbage hole for all kinds of things, stack (0x2fffffff->down), bss main program (0x20000000->up), heap in small model (0x20000000+bss->up). Also see heap small and huge below.

3) heap outjson (malloc/free) - range 0x20000000 - 0x2fffffff in default small mode (yep share with stack and they grow toward each other .. tee hee ... can you say train head on collision). Or range 0x30000000 - 0xAfffffff in huge model depending on compile or loader control flags for huge memory heap allocation.

4) shmat/mmap outjson (not shown) - ranger -0xAffffffff to 0x300000000 (yep, huge heap and shared memory grow toward each other ... toot, toot, train on track 9).

Just for completion ...

a) text/instructions (running program) - range 0x10000000 to 0x1fffffff. Processors run your main program.

b) text/instructions (shared objects like libc.a, or thing.so) - range 0xd0000000 to 0xdfffffff. Processor runs your shared library code (libc.a functions). Main to shared function calls resolved by kernel loader (at load time). Linuz resolves at run time (difference AIX/PASE to Linux). The loads/resolves can be dynamically controlled via the 'dl' functions to match Linux-like (dlopen, dlsym, dlclose, etc.)

c) kernel heap (protected) - range 0x00000000 - 0x0fffffff. PASE loader heap, various fast running test/instruction blas (memcpy, etc), system call vector table, so on. All protected as read only or no access.

If you want to see the full address layout of 32 bit and 64 bit, i put a copy on Yips. Frankly, this is how PASE experts like me can look at an address and practically tell your programming DNA of how it formed (not that we are invincible) .

Ok, you are a c programmer now.

BTW - recall ... you started the memory discussion ... i am just an old guy knows lot's about c programming with heart of a teacher. Also, no idea if this example program was your real 'memory question (bit a toy program, i think).

kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


more unsolicited ...

I don't really know what you are doing, so assuming above example is a 'toy'. However to give you a leg up on c programming. A static/global like outjson is not thread safe without mutex to avoid concurrent threads updates. To wit, if you are using this static 'toy' idea in a language like node with async for a 'driver', the static will corrupted on multiple requests, aka, two or more threads update at same time without mutex locks. If you want to be thread safe with a trial design (toy), simply move outjson on the stack (instead of static/global).

BTW -- You still need to memset (zero) outjson on stack to avoid old data left in various stack invocations. That is, stack grows downward toward zero (toward 0x20000000 in 32 bit), but used stack chunks are not zero'd after returned, so junk can be in outjson on stack. Aka, good c programming is initializing stack variables (memset outjson). Or ... when not done (forgot memset) ... how to catch a beginning c programmer with 'random' problems (that can't be explained to the boss).

#!c

void calljson()
{
  char outjson[1000000]; /* on stack (safe but may blow stack size) */
  SQLINTEGER inlen = strlen(injson);
  outjson[0] = 0;
  SQLINTEGER outlen = sizeof(outjson);
  memset(outjson,0,sizeof(outjson)); /* zero any previous stack junk */
  SQL400Json(0, injson, inlen, outjson, outlen);
  printf("result %s\n", outjson);
}

If you really mean to share a static outjson, in threaded environment you will need to understand mutex locking (db2sock has examples).

Thus ends the c code 101 lesson of the day.

kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


You are using a static variable for outjson (global c variable). There is nothing to free. Aka, you are not using malloc/free at all.

You appear new to c programming. Am i clear in response? Do you understand c static/global 'data'?

kadler commented 6 years ago

Original comment by Tony Cairns (Bitbucket: rangercairns, GitHub: rangercairns).


Looks like it doesn't free memory or did I miss something obvious?

You are using a static variable for outjson (global c variable). There is nothing to free. Aka, you are not using malloc/free at all.

#!c

char outjson[1000000]; <--- static variable (bss technical term)

void calljson()
{
  SQLINTEGER inlen = strlen(injson);
  outjson[0] = 0;
  SQLINTEGER outlen = sizeof(outjson);
  SQL400Json(0, injson, inlen, outjson, outlen);
  printf("result %s\n", outjson);
}

Unsolicited more ...

Traditionally when re-using a static variable (loop), convention often holds to zero the memory between uses. Zeroing avoids left over json from a bigger size return in previous loop.

#!c

char outjson[1000000]; <--- static variable (bss technical term)

void calljson()
{
  SQLINTEGER inlen = strlen(injson);
  outjson[0] = 0;
  SQLINTEGER outlen = sizeof(outjson);
  memset(outjson,0,sizeof(outjson)); /* zero any previous output */
  SQL400Json(0, injson, inlen, outjson, outlen);
  printf("result %s\n", outjson);
}