Opened 10 years ago

Closed 9 years ago

#214 closed defect (fixed)

ASIO fix causes auth not to listen to commands/config updates

Reported by: each Owned by: jelte
Priority: high Milestone:
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket:
Total Hours: Internal?: no

Description

In #168 we converted b10-auth to use asio rather than boost::asio or the home-grown select-based query response loop. When I merged this I noticed that the home-grown version had a capability the boost::asio version (and the new asio version that was derived from it) lacked: monitoring for queued configuration commands. I tried to copy that logic into the asio version and apparently did it wrong.

Taking the liberty of assigning this to Jelte even though the bug was my fault, 'cause he knows the config system wayyyy better than me...

Jelte's comment from #168:

This doesn't seem exactly right, the hasQueuedMessages&&checkCommand was used in before select() loops where the processing of the loop could have had synchronous request/response communication. If there are other messages arriving while this is being done these are queued and need to be resolved at some point. In the loop structure the right place for this is at the start of the loop. In an asynchronous setup, I'm not entirely sure whether this should be handled at all, though I haven't fully wrapped my head around asio yet (do we get an immediate calling of the io_services handler the moment a new message arrives, even if we're still busy handling other stuff? Or does asio call it as soon as we're done, at which point we do have possibly queued messages, and if so the current way is probably right, although we could consider putting it in moduleccsession itself, at the end of checkcommand (while stuff_left handle_stuff)).

There is, however, a more immediate problem; in the current trunk, commands aren't handled at all. Since ModuleCCSession is started without an io_service, it defaults to the (non-asio) SocketSession??, which expects manual polls. I have a small patch that fixes this, but it needs expansion of the asio_link API, to get access to the underlying io_service object. This isn't really clean and I think we should probably refactor this, though I'm not sure how, and i'd like to have a bit of a brainstorm for this (and probably another ticket).

Subtickets

Attachments (1)

bind10_auth_asio_ccsession.patch (1.6 KB) - added by jelte 10 years ago.
Here's the patch that at least fixes the symptoms, see the description :)

Download all attachments as: .zip

Change History (2)

Changed 10 years ago by jelte

Here's the patch that at least fixes the symptoms, see the description :)

comment:1 Changed 9 years ago by jelte

  • billable set to 0
  • Estimated Difficulty set to 0.0
  • Internal? unset
  • Resolution set to fixed
  • Status changed from new to closed

this patch has been committed back in r2007, and i think we left the ticket open for some other refactoring issues, which seem either irrelevant now (for the purpose of this ticket), or already handled (as a side-effect of #221?).

In any case, I don't think it makes sense to leave this ticket open any longer, and I am closing it. If someone feels we need more refactoring here at this point, please open up another ticket with specific suggestions.

Note: See TracTickets for help on using tickets.