Opened 10 months ago

Last modified 10 months ago

#2433 new defect

Refactor SSU EstablishmentManager doPass()

Reported by: zzz Owned by: zzz
Priority: minor Milestone: undecided
Component: router/transport Version: 0.9.38
Keywords: Cc: Zlatin Balevsky, jogger
Parent Tickets: Sensitive: no

Description

As discovered when working on #2397

  • See if failsafe stuff ever fires or is necessary
  • Increase 1 sec sleep to much longer once verified everything is working
  • handleInbound() and handleOutbound() should drive completely through the iterator rather than returning now() and forcing a second call
  • activityLock and the activity integer can be combined into some sort of concurrent lock or boolean or something
  • shortcuts when inboundStates or outboundStates is empty

Not super important because the number of states is usually small, this is only for establishment. This is very old code from jrandom, hasn't gotten significant cleanup in 15 years

Subtickets

Change History (1)

comment:1 Changed 10 months ago by jogger

I have looked through this and will provide a cleanup. I would not change much as 99% of time is spent in crypto functions.

Notable:

  • the loop intermixes lengthy crypto with timeout checks. As this thread consumes quite some CPU at router startup on slower machines, this is a potential reliability problem and the real improvement would be to run the crypto in a separate thread through the job queue or equivalent
  • because of this thought must be given to order of execution and placement of calls to the clock
  • The current code as it works strips out fails and otherwise alternates between one inbound, one outbound and failsafe. Sounds reasonable for startup.
  • The activity integer guards against the common mistake I fixed with _targetPeer in #2412 and _threadSafeWait in #2434. Will make cleaner.
Note: See TracTickets for help on using tickets.