Skip to content

Conversation

jrw972
Copy link
Contributor

@jrw972 jrw972 commented Aug 16, 2022

Problem

Entries in the maps of DataWriterImpl_t and WriteDataContainer
correspond to instances. Unregistering an instance should remove from
these maps to prevent a memory leak.

Solution

Remove from these maps when instances are unregistered.

@jrw972 jrw972 self-assigned this Aug 16, 2022
@jrw972 jrw972 force-pushed the unregister-instance branch from cb779a9 to c10703e Compare August 16, 2022 20:23
@jrw972 jrw972 force-pushed the unregister-instance branch from c10703e to fa655aa Compare August 16, 2022 21:37
@jwillemsen
Copy link
Member

Any explicit unit test possible for this?

Problem
-------

Entries in the maps of DataWriterImpl_t and WriteDataContainer
correspond to instances.  Unregistering an instance should remove from
these maps to prevent a memory leak.

Solution
--------

Remove from these maps when instances are unregistered.
@jrw972 jrw972 force-pushed the unregister-instance branch from fa655aa to 33092fe Compare August 17, 2022 13:37
@jrw972
Copy link
Contributor Author

jrw972 commented Aug 17, 2022

Any explicit unit test possible for this?

Possible yes. However, I think it's going to take a serious refactoring of DataWriterImpl_T, DataWriterImpl, WriteDataContainer, and the associated transport framework to write it as a unit test and not an integration test.

@mitza-oci
Copy link
Member

Any explicit unit test possible for this?

FooTest3_0 is not exactly a unit test, but it will provide coverage of these code paths

@mitza-oci mitza-oci merged commit 91fb793 into OpenDDS:master Aug 18, 2022
mitza-oci added a commit that referenced this pull request Aug 22, 2022
DataWriter doesn't remove unregistered instances

(cherry picked from commit 91fb793)

# Conflicts:
#	dds/DCPS/DataWriterImpl_T.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants