Skip to content

Json mem leak#82

Merged
koolzz merged 5 commits intosdnfv:developfrom
kevindweb:json_mem_leak
Mar 17, 2019
Merged

Json mem leak#82
koolzz merged 5 commits intosdnfv:developfrom
kevindweb:json_mem_leak

Conversation

@kevindweb
Copy link
Copy Markdown
Contributor

@kevindweb kevindweb commented Mar 13, 2019

Fixes onvm web memory leak by deleting the cJSON objects upon exiting

Summary:

In order to avoid data leaks after running the web console of ONVM, cJSON objects initialized in onvm_stats.c needed to be freed. cJSON objects are created and destroyed with helper functions from /onvm/lib/cJSON.c. The code in onvm_stats_cleanup fixes this bug.

Usage:

This PR includes
Resolves issues Fixes #283
Breaking API changes
Internal API changes
Usability improvements
Bug fixes 👍
New functionality
New NF/onvm_mgr args
Changes to starting NFs
Dependency updates
Web stats updates 👍

Merging notes:

  • Dependencies: None

TODO before merging :

  • PR is ready for review

Test Plan:

Tested on a Wisconsin c220g2 cloudlab node with Ubuntu 14.04.1
Run using onvm web as the output - onvm_stats_cleanup code doesn't run on the standard onvm stdout

Speed Tester Packets/Sec
1 NF 42097232

Review:

  • Sanity checks, assigned to @koolzz @dennisafa
    • Run make in /onvm and /examples directories
    • Test new functionality or bug fix from the pr (if needed)
  • Code style, assigned to @koolzz @dennisafa
    • Run linter
    • Check everything style related
  • Code design, assigned to @koolzz @dennisafa
    • Check for memory leaks
    • Check that code is reusable
    • Code is clean, functions are concise
    • Verify that edge cases properly handled
  • Performance, assigned to @koolzz @dennisafa
    • Run onvm with the web output (<onvm go args> -s web)
    • Run Speed Tester NF, report performance.
  • Documentation, assigned to @dennisafa
    • Check if the new changes are well documented, in both code and READMEs

@koolzz koolzz self-requested a review March 13, 2019 23:24

/************************Internal Functions Prototypes************************/


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the style is 2 blank line after the /** **/ declaration (even if it isn't lets not change only a few occurences)


/****************************Internal functions*******************************/


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same style nit as above

Copy link
Copy Markdown
Member

@koolzz koolzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix, just some style nits but overall approved.

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 17, 2019

@onvm do your thing

@onvm
Copy link
Copy Markdown

onvm commented Mar 17, 2019

@onvm do your thing

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented Mar 17, 2019

@onvm do your thing

CI Message

Run successful see results:
[Results from nimbnode30]
Median TX pps for Speed Tester: 35117126

Linter Failed

examples/aes_decrypt/aes.c:523: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_decrypt/aes.c:1024: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_decrypt/aes.c:1028: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_decrypt/aes.c:1081: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_decrypt/aes.c:1087: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 5
examples/aes_encrypt/aes.c:523: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_encrypt/aes.c:1024: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_encrypt/aes.c:1028: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_encrypt/aes.c:1081: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_encrypt/aes.c:1087: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 5
examples/arp_response/arp_response.c:144: Missing space before ( in switch( [whitespace/parens] [5]
Total errors found: 1
examples/bridge/bridge.c:135: Missing space before ( in if( [whitespace/parens] [5]
examples/bridge/bridge.c:138: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/bridge/bridge.c:156: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 3
examples/flow_table/flow_table.c:103: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/flow_table.c:198: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/flow_table.c:216: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/flow_table.c:224: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/flow_table.c:231: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/flow_table.c:238: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 6
examples/flow_table/msgbuf.c:71: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:73: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:83: Missing space before ( in while( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:86: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:89: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:106: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:108: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:116: Missing space before ( in while( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:119: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:125: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:138: Extra space before last semicolon. If this should be an empty statement, use {} instead. [whitespace/semicolon] [5]
examples/flow_table/msgbuf.c:140: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:149: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:157: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:159: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:162: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:169: Missing space before ( in while( [whitespace/parens] [5]
Total errors found: 17
examples/flow_table/sdn.c:99: Extra space before last semicolon. If this should be an empty statement, use {} instead. [whitespace/semicolon] [5]
examples/flow_table/sdn.c:125: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:127: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:147: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:147: Missing space before { [whitespace/braces] [5]
examples/flow_table/sdn.c:153: Missing space before ( in while( [whitespace/parens] [5]
examples/flow_table/sdn.c:160: Missing space before { [whitespace/braces] [5]
examples/flow_table/sdn.c:235: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:239: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:279: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:418: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:423: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:493: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:501: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:538: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:542: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:545: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:547: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:555: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:569: Missing space before ( in while( [whitespace/parens] [5]
Total errors found: 20
examples/flow_table/setupconn.c:82: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:89: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:94: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:102: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:110: Missing space before { [whitespace/braces] [5]
examples/flow_table/setupconn.c:112: Missing space before { [whitespace/braces] [5]
examples/flow_table/setupconn.c:122: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:140: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:149: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:156: Missing space before ( in if( [whitespace/parens] [5]
Total errors found: 10
examples/flow_tracker/flow_tracker.c:108: Missing space before ( in switch( [whitespace/parens] [5]
examples/flow_tracker/flow_tracker.c:210: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 2
examples/scaling_example/scaling.c:312: Missing space before ( in while( [whitespace/parens] [5]
Total errors found: 1
onvm/onvm_mgr/main.c:98: Mismatching spaces inside () in while [whitespace/parens] [5]
Total errors found: 1
onvm/onvm_mgr/onvm_args.c:166: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_args.c:166: Missing space before { [whitespace/braces] [5]
onvm/onvm_mgr/onvm_args.c:172: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_args.c:172: Missing space before { [whitespace/braces] [5]
onvm/onvm_mgr/onvm_args.c:328: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_args.c:352: Missing space before { [whitespace/braces] [5]
onvm/onvm_mgr/onvm_args.c:357: Missing space before ( in if( [whitespace/parens] [5]
Total errors found: 7
onvm/onvm_mgr/onvm_init.c:222: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
Total errors found: 1
onvm/onvm_mgr/onvm_nf.c:182: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_nf.c:255: Missing space before ( in if( [whitespace/parens] [5]
Total errors found: 2
onvm/onvm_mgr/onvm_stats.c:186: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_stats.c:186: Missing space before { [whitespace/braces] [5]
Total errors found: 2
onvm/onvm_nflib/onvm_config_common.c:52: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_config_common.c:112: Missing space before ( in for( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_config_common.c:112: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_config_common.c:417: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
onvm/onvm_nflib/onvm_config_common.c:433: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
onvm/onvm_nflib/onvm_config_common.c:487: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
Total errors found: 6
onvm/onvm_nflib/onvm_flow_dir.c:64: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_flow_dir.c:95: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:103: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:111: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:128: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:143: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:151: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:159: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:176: Missing space before { [whitespace/braces] [5]
Total errors found: 9
onvm/onvm_nflib/onvm_nflib.c:341: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:344: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:347: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:351: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:354: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:357: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:584: Missing space before ( in switch( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:743: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:754: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:821: Missing space before { [whitespace/braces] [5]
Total errors found: 10
onvm/onvm_nflib/onvm_pkt_helper.c:313: Missing space before ( in switch( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_pkt_helper.c:376: Missing space before { [whitespace/braces] [5]
Total errors found: 2

@koolzz koolzz merged commit 6f1e725 into sdnfv:develop Mar 17, 2019
@koolzz koolzz added this to the ONVM 19.05 Release milestone May 26, 2019
@kevindweb kevindweb deleted the json_mem_leak branch June 1, 2019 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants