Opened 4 years ago

Closed 4 years ago

#17724 closed defect (fixed)

Unreliable rend_cache_purge test

Reported by: cypherpunks Owned by:
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Minor Keywords: TorCoreTeam201512, tests
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The test_rend_cache_purge function contains an assertion which verifies that the internal strmap does not change after a purge (src/test/test_rendcache.c:1044 on 0a701e537778ac9da31049f4efebf7cb2bf9c285).

However, the rend_cache_purge function frees the internal strmap and allocates a new one. This turns the assertion in a check whether memory allocation returns the same address as was just freed. The C11 standard mentions that a previous call to free is synchronized with a call to malloc [0].

I found the issue during a Valgrind run which probably changed the behavior of free and malloc to track them and caused the assertion to fail.

I am suggesting to remove the assertion because it verifies behavior that does not affect normal operation and (in my case) interferes with testing.

[0] http://en.cppreference.com/w/c/memory/malloc

Child Tickets

Change History (2)

comment:1 in reply to:  description Changed 4 years ago by teor

Keywords: TorCoreTeam201512 tests added
Milestone: Tor: 0.2.8.x-final
Status: newneeds_review

Replying to cypherpunks:

The test_rend_cache_purge function contains an assertion which verifies that the internal strmap does not change after a purge (src/test/test_rendcache.c:1044 on 0a701e537778ac9da31049f4efebf7cb2bf9c285).

However, the rend_cache_purge function frees the internal strmap and allocates a new one. This turns the assertion in a check whether memory allocation returns the same address as was just freed. The C11 standard mentions that a previous call to free is synchronized with a call to malloc [0].
...
I am suggesting to remove the assertion because it verifies behavior that does not affect normal operation and (in my case) interferes with testing.

Relying on the internals of malloc in unit tests is unhelpful.

Please see my branch bug17724 in https://github.com/teor2345/tor.git

comment:2 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

okay, merged!

Note: See TracTickets for help on using tickets.