[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #18362 [Core Tor/Tor]: Tor could use a generic 'handle' implementation.
#18362: Tor could use a generic 'handle' implementation.
-------------------------------------------------+-------------------------
Reporter: nickm | Owner: nickm
Type: enhancement | Status:
Priority: High | needs_revision
Component: Core Tor/Tor | Milestone: Tor:
Severity: Normal | 0.2.9.x-final
Keywords: modularity, TorCoreTeam201604, tor- | Version:
modularity | Resolution:
Parent ID: | Actual Points:
Reviewer: dgoulet | Points: small
| Sponsor:
| SponsorS-can
-------------------------------------------------+-------------------------
Changes (by dgoulet):
* status: needs_review => needs_revision
Comment:
Some review comments:
* In `_handle_new`, I don't think the two lines below need to be in the
critical section. How I understand it, the mutex is used to protect the
`head` object (handle_head) but the following doesn't do memory access on
it.
{{{
name ## _handle_t *new_ref = tor_malloc_zero(sizeof(*new_ref));
new_ref->head = head;
}}}
* Probably all refcount increment/decrement could be done atomically which
would remove the locking in the `_handle_new` function (small
improvement).
General comment. I think we could achieve this without locking (only
atomic op). I find it a bit weird to use locking in something that in a
multi-thread context would not be very useful since it doesn't give any
guarantee on the validity of `object` pointer. In that context, even if
`_handle_get` sends me back a non NULL pointer, I have no clue if that
pointer is usable or not just after.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18362#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs