Memory leaks in CSSParser, HTMLParser and DocumentNode classes #35

Open
opened 1 year ago by ncc1988 · 7 comments
ncc1988 commented 1 year ago
Owner

Recent executions of the mnestripper tool (from moenavigatonengine-tools) showed that there are memory leaks in the CSSParser, HTMLParser and DocumentNode classes. Here is the output of one example execution of mnestripper:

3 bytes in 1 blocks are definitely lost in loss record 1 of 23                                                                                                            
[...]
CSSParser::decodeColourValues(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (CSSParser.cpp:700)                       
by:
CSSParser::convertAttribute(std::shared_ptr<StylesheetAttribute>, VisualAttributes&) (CSSParser.cpp:793)                                                 
by: CSSParser::parseFromString(char const*, unsigned long, unsigned char) (CSSParser.cpp:1269)                                                               
by: CSSParser::parseFromString(char const*, unsigned long) (CSSParser.cpp:871)                                                                               

The same message also appeared for CSSParser.cpp line 701 and 702.

31 bytes in 1 blocks are definitely lost in loss record 7 of 23
[...]
HTMLParser::parseData(std::shared_ptr<std::vector<unsigned char, std::allocator<unsigned char> > >) (HTMLParser.cpp:474)
by: MoeNavigatorEngine::openURL(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (MoeNavigatorEngine.cpp:791)
7,036 (552 direct, 6,484 indirect) bytes in 1 blocks are definitely lost in loss record 22 of 23
[...]
by: HTMLParser::parseData(std::shared_ptr<std::vector<unsigned char, std::allocator<unsigned char> > >) (HTMLParser.cpp:550)
[...]
8,156 (552 direct, 7,604 indirect) bytes in 1 blocks are definitely lost in loss record 23 of 23
[...]
by: DocumentNode::cloneNode(bool) (DocumentNode.cpp:527)
by: HTMLParser::returnNodeTree() (HTMLParser.cpp:727)
[...]
Recent executions of the mnestripper tool (from moenavigatonengine-tools) showed that there are memory leaks in the CSSParser, HTMLParser and DocumentNode classes. Here is the output of one example execution of mnestripper: ``` 3 bytes in 1 blocks are definitely lost in loss record 1 of 23 [...] CSSParser::decodeColourValues(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (CSSParser.cpp:700) by: CSSParser::convertAttribute(std::shared_ptr<StylesheetAttribute>, VisualAttributes&) (CSSParser.cpp:793) by: CSSParser::parseFromString(char const*, unsigned long, unsigned char) (CSSParser.cpp:1269) by: CSSParser::parseFromString(char const*, unsigned long) (CSSParser.cpp:871) ``` The same message also appeared for CSSParser.cpp line 701 and 702. ``` 31 bytes in 1 blocks are definitely lost in loss record 7 of 23 [...] HTMLParser::parseData(std::shared_ptr<std::vector<unsigned char, std::allocator<unsigned char> > >) (HTMLParser.cpp:474) by: MoeNavigatorEngine::openURL(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (MoeNavigatorEngine.cpp:791) ``` ``` 7,036 (552 direct, 6,484 indirect) bytes in 1 blocks are definitely lost in loss record 22 of 23 [...] by: HTMLParser::parseData(std::shared_ptr<std::vector<unsigned char, std::allocator<unsigned char> > >) (HTMLParser.cpp:550) [...] ``` ``` 8,156 (552 direct, 7,604 indirect) bytes in 1 blocks are definitely lost in loss record 23 of 23 [...] by: DocumentNode::cloneNode(bool) (DocumentNode.cpp:527) by: HTMLParser::returnNodeTree() (HTMLParser.cpp:727) [...] ```
ncc1988 added this to the Version 0.0.1 - "Uyghur lives matter!" milestone 1 year ago
ncc1988 added the
bug
TODO
labels 1 year ago
ncc1988 self-assigned this 1 year ago
ncc1988 added the
DOING
label 1 year ago
ncc1988 referenced this issue from a commit 1 year ago
Poster
Owner

CSSParser is fixed.

CSSParser is fixed.
ncc1988 changed title from Memory leaks in CSSParser and HTMLParser classes to Memory leaks in CSSParser, HTMLParser and DocumentNode classes 1 year ago
ncc1988 removed the
TODO
label 11 months ago
Poster
Owner

The problem in DocumentNode occurs if a DocumentNode is constructed (via std::make_shared<DocumentNode>()) and then calling appendChild with another shared_ptr<DocumentNode>.

The problem in DocumentNode occurs if a DocumentNode is constructed (via std::make_shared\<DocumentNode\>()) and then calling appendChild with another shared_ptr\<DocumentNode\>.
Poster
Owner

The cause: A circular reference:

When parent_node of the child is set in DocumentNode::appendChild (via shared_from_this), the child and the parent own each other.

The cause: A circular reference: When parent_node of the child is set in DocumentNode::appendChild (via shared_from_this), the child and the parent own each other.
Poster
Owner

More circular references: previous and next pointers.

More circular references: previous and next pointers.
Poster
Owner

DocumentNode is fixed.

DocumentNode is fixed.
Poster
Owner

Using a real-world example page, more memory leaks have been found, related to HTMLParser.

Using a real-world example page, more memory leaks have been found, related to HTMLParser.
Poster
Owner

All the evidence points to the DocumentNode class as the source of the remaining memory leaks. There may be still some circular references left in that class.

All the evidence points to the DocumentNode class as the source of the remaining memory leaks. There may be still some circular references left in that class.
ncc1988 referenced this issue from a commit 3 months ago
Sign in to join this conversation.
Loading…
There is no content yet.