microsoft / microsoft/react-native-windows
Fix and reenable Facebook's MapBuffer code
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 17.3k
- Forks
- 1.2k
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 33
Description
The MapBuffer class has a double-free bug in the code that makes the unittests crash.
For now we have disabled the unittests (See .ado\jobs\desktop.yml).
The problem is that MapBufferBuilder in the build() method stack allocates the class that does not use proper std:: pointer semantics...
So the MapBuffer class is destructed at the end of the method which delete's it's member _data.
The test code just uses the stack allocated value tests it and at the end of the test method i.e. testSimpleIntMap the destructor is called again double-freeing the _data member.
There is also a crash hiding where the v8 isolate is disposed on the wrong thread... Perhaps this is caused by the crash in the MapBuffer object... Not sure. Should be investigated before reenabeling this test though...
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with MapBufferBuilder's build() method and the MapBuffer usage in testSimpleIntMap; the disabled test configuration is in .ado\jobs\desktop.yml. Confirm the ownership bug and investigate the V8 isolate disposal thread issue before re-enabling the unittests; done means the tests no longer crash and are enabled again.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, react-native
- Domain
- desktop, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100