Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug 4207 #157

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Bug 4207 #157

wants to merge 15 commits into from

Conversation

milan-mpathix
Copy link
Contributor

Bug fix for Bug 4207:

  • added test case exposing the bug
  • fixed memory leak in TAO/tao/Cache_Entries
  • fixed semantics of ACE_Multi_Hash_Map_Manager, to preserve pointers of
    existing elements when new elements are added or removed
  • Transport_Cache_Manager uses multi-hash map instead of map of indexed elements
  • removed index_ from Cache_ExtId

@milan-mpathix
Fix memory leak when copying Cache_ExtId
4db0fd2
@milan-mpathix
Test case exposing Bug 4207
66a890f
@milan-mpathix
Hash_Multi_Map_Manager fixes
12a27ff 
- preserve pointer to elements when inserting second element with same
  hash value
- properly count current_size
@milan-mpathix
Transport_Cache_Manager uses ACE_Hash_Multi_Map_Manager
c075921 
In order to keep a reference to specific entry in ACE_Hash_Multi_Map_Manager
Transport keeps a copy of ACE_Hash_Multi_Map_Manager::HASH_MAP_ENTRY*
and a pointer to a specific value corelated to the transport.

- update TAO/tests/Transport_Cache_Manager
- update TAO/tests/Bug_1476_Regression
- update TAO/orbsvcs/tests/Security/MT_IIOP_SSL
@milan-mpathix
Remove unused Cache_ExtId::index_
2439ed4
jwillemsen
jwillemsen reviewed Nov 16, 2015
View reviewed changes
TAO/tao/Cache_Entries_T.inl Outdated
@@ -128,6 +126,9 @@ namespace TAO
{
if (this != &rhs)
{
if (this->is_delete_)
delete this->transport_property_;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this->transport_property_ be set to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The very next line in the same file assigns the value to this->transport_property_. I will add setting to 0 to stay exception neutral

@jwillemsen
Copy link
Member

Have you compiled and ran all TAO unit tests? That is something we can't do on travis/snap, maybe you can setup a branch build to show that all tests do work without problems at runtime? This change is in the core of TAO and it could be that there are unforeseen issues.

@milan-mpathix
Copy link
Contributor Author

I will re-run all the tests, and verify it compiles/runs with versioned namespaces.

@milan-mpathix
Copy link
Contributor Author

@jwillemsen (setup a branch build) .... not sure how to do this...

What is the simplest way to post build/test results. I will run $ACE_ROOT/bin/auto_run_tests.pl and capture all the output, but the resulting file tends to be quite large...

@jwillemsen
Copy link
Member

Check the scoreboard for existing builds, the autobuild repo has all tools and scripts we use

@milan-mpathix
Copy link
Contributor Author

Please see the scoreboard:
http://support.mpathix.com/tao_scoreboard/index.html

@milan-mpathix
Copy link
Contributor Author

All patches for this pull request are complete. Please review.

@jwillemsen jwillemsen added the needs review Needs to be reviewed label Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs review Needs to be reviewed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@milan-mpathix @jwillemsen