Fix concurrency safety bugs which make overture slow in some circumst…

…ances

config.sample.json

@@ -27,7 +27,7 @@
     }
   ],
   "OnlyPrimaryDNS": false,
-  "RedirectIPv6Record": true,
+  "RedirectIPv6Record": false,
   "IPNetworkFile": "./ip_network_sample",
   "DomainFile": "./domain_sample",
   "DomainBase64Decode": true,

config.test.json

@@ -32,7 +32,7 @@
   "DomainFile": "./domain_sample",
   "DomainBase64Decode": true,
   "HostsFile": "./hosts_sample",
-  "MinimumTTL": 0,
-  "CacheSize" : 100000,
+  "MinimumTTL": 100000,
+  "CacheSize" : 604800,
   "RejectQtype": [255]
-}
+}

core/common/common.go

@@ -15,12 +15,11 @@ import (
 
 var ReservedIPNetworkList []*net.IPNet
 
-func init(){
+func init() {
 
 	ReservedIPNetworkList = getReservedIPNetworkList()
 }
 
-
 func IsIPMatchList(ip net.IP, ipnl []*net.IPNet, isLog bool) bool {
 
 	for _, ip_net := range ipnl {
@@ -35,14 +34,7 @@ func IsIPMatchList(ip net.IP, ipnl []*net.IPNet, isLog bool) bool {
 	return false
 }
 
-func IsAnswerEmpty(m *dns.Msg) bool {
-
-	if len(m.Answer) == 0 {
-		return true
-	}
-
-	return false
-}
+func HasAnswer(m *dns.Msg) bool { return len(m.Answer) != 0 }
 
 func HasSubDomain(s string, sub string) bool {
 
@@ -73,4 +65,4 @@ func FindRecordByType(msg *dns.Msg, t uint16) string {
 	}
 
 	return ""
-}
+}

core/config/config.go

@@ -17,8 +17,8 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/shawn1m/overture/core/cache"
-	"github.com/shawn1m/overture/core/hosts"
 	"github.com/shawn1m/overture/core/common"
+	"github.com/shawn1m/overture/core/hosts"
 )
 
 type Config struct {

core/inbound/server.go

@@ -8,21 +8,16 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/miekg/dns"
-	"github.com/shawn1m/overture/core/cache"
-	"github.com/shawn1m/overture/core/hosts"
 	"github.com/shawn1m/overture/core/outbound"
 )
 
 type Server struct {
 	BindAddress string
 
-	Dispatcher *outbound.Dispatcher
+	Dispatcher outbound.Dispatcher
 
 	MinimumTTL  int
 	RejectQtype []uint16
-
-	Hosts *hosts.Hosts
-	Cache *cache.Cache
 }
 
 func (s *Server) Run() {
@@ -51,8 +46,8 @@ func (s *Server) Run() {
 func (s *Server) ServeDNS(w dns.ResponseWriter, q *dns.Msg) {
 
 	inboundIP, _, _ := net.SplitHostPort(w.RemoteAddr().String())
-	s.Dispatcher.PrimaryClientBundle = outbound.NewClientBundle(q, s.Dispatcher.PrimaryDNS, inboundIP, s.Hosts, s.Cache)
-	s.Dispatcher.AlternativeClientBundle = outbound.NewClientBundle(q, s.Dispatcher.AlternativeDNS, inboundIP, s.Hosts, s.Cache)
+	s.Dispatcher.InboundIP = inboundIP
+	s.Dispatcher.QuestionMessage = q
 
 	log.Debug("Question: " + q.Question[0].String())
 
@@ -62,9 +57,11 @@ func (s *Server) ServeDNS(w dns.ResponseWriter, q *dns.Msg) {
 		}
 	}
 
-	s.Dispatcher.Exchange()
+	d := s.Dispatcher
+
+	d.Exchange()
 
-	cb := s.Dispatcher.ActiveClientBundle
+	cb := d.ActiveClientBundle
 
 	if cb.ResponseMessage != nil {
 		if s.MinimumTTL > 0 {

core/init.go

@@ -6,9 +6,9 @@
 package core
 
 import (
+	"github.com/shawn1m/overture/core/config"
 	"github.com/shawn1m/overture/core/inbound"
 	"github.com/shawn1m/overture/core/outbound"
-	"github.com/shawn1m/overture/core/config"
 )
 
 // Initiate the server with config file
@@ -17,22 +17,22 @@ func InitServer(configFilePath string) {
 	config := config.NewConfig(configFilePath)
 
 	// New dispatcher without ClientBundle, ClientBundle must be initiated when server is running
-	d := &outbound.Dispatcher{
+	d := outbound.Dispatcher{
 		PrimaryDNS:         config.PrimaryDNS,
 		AlternativeDNS:     config.AlternativeDNS,
 		OnlyPrimaryDNS:     config.OnlyPrimaryDNS,
 		IPNetworkList:      config.IPNetworkList,
 		DomainList:         config.DomainList,
 		RedirectIPv6Record: config.RedirectIPv6Record,
+		Hosts:              config.Hosts,
+		Cache:              config.Cache,
 	}
 
 	s := &inbound.Server{
 		BindAddress: config.BindAddress,
 		Dispatcher:  d,
 		MinimumTTL:  config.MinimumTTL,
 		RejectQtype: config.RejectQtype,
-		Hosts:       config.Hosts,
-		Cache:       config.Cache,
 	}
 
 	s.Run()

core/outbound/client.go

@@ -16,7 +16,7 @@ import (
 
 type Client struct {
 	ResponseMessage *dns.Msg
-	QuestionMessage dns.Msg
+	QuestionMessage *dns.Msg
 
 	DNSUpstream        *common.DNSUpstream
 	EDNSClientSubnetIP string
@@ -28,7 +28,7 @@ type Client struct {
 
 func NewClient(q *dns.Msg, u *common.DNSUpstream, ip string, h *hosts.Hosts, cache *cache.Cache) *Client {
 
-	c := &Client{QuestionMessage: *q, DNSUpstream: u, InboundIP: ip, Hosts: h, Cache: cache}
+	c := &Client{QuestionMessage: q.Copy(), DNSUpstream: u, InboundIP: ip, Hosts: h, Cache: cache}
 
 	c.getEDNSClientSubnetIP()
 	return c
@@ -49,8 +49,8 @@ func (c *Client) getEDNSClientSubnetIP() {
 
 func (c *Client) ExchangeFromRemote(isCache bool, isLog bool) {
 
-	common.SetEDNSClientSubnet(&c.QuestionMessage, c.EDNSClientSubnetIP)
-	c.EDNSClientSubnetIP = common.GetEDNSClientSubnetIP(&c.QuestionMessage)
+	common.SetEDNSClientSubnet(c.QuestionMessage, c.EDNSClientSubnetIP)
+	c.EDNSClientSubnetIP = common.GetEDNSClientSubnetIP(c.QuestionMessage)
 
 	var conn net.Conn
 	if c.DNSUpstream.SOCKS5Address != "" {
@@ -80,7 +80,7 @@ func (c *Client) ExchangeFromRemote(isCache bool, isLog bool) {
 
 	dc := &dns.Conn{Conn: conn}
 	defer dc.Close()
-	err := dc.WriteMsg(&c.QuestionMessage)
+	err := dc.WriteMsg(c.QuestionMessage)
 	if err != nil {
 		log.Warn(c.DNSUpstream.Name + " Fail: Send question message failed")
 		return
@@ -100,6 +100,16 @@ func (c *Client) ExchangeFromRemote(isCache bool, isLog bool) {
 
 	c.ResponseMessage = temp
 
+	//for i := 0;i < len(c.ResponseMessage.Answer); i++{
+	//	if c.ResponseMessage.Answer[i].Header().Rrtype == dns.TypeA || c.ResponseMessage.Answer[i].Header().Rrtype == dns.TypeAAAA{
+	//		c.ResponseMessage.Answer[i].Header().Name = c.QuestionMessage.Question[0].Name
+	//	}
+	//	if c.ResponseMessage.Answer[i].Header().Rrtype == dns.TypeCNAME{
+	//		c.ResponseMessage.Answer = c.ResponseMessage.Answer[:i+copy(c.ResponseMessage.Answer[i:], c.ResponseMessage.Answer[i+1:])]
+	//		i -= 1
+	//	}
+	//}
+
 	if isLog {
 		c.logAnswer("")
 	}
@@ -209,7 +219,7 @@ func (c *Client) setLocalResponseMessage(rrl []dns.RR) {
 		c.ResponseMessage.Answer = append(c.ResponseMessage.Answer, rr)
 	}
 	shuffleRRList(c.ResponseMessage.Answer)
-	c.ResponseMessage.SetReply(&c.QuestionMessage)
+	c.ResponseMessage.SetReply(c.QuestionMessage)
 	c.ResponseMessage.RecursionAvailable = true
 }
 
@@ -229,6 +239,6 @@ func (c *Client) logAnswer(indicator string) {
 func (c *Client) CacheResult() {
 
 	if c.Cache != nil {
-		c.Cache.InsertMessage(cache.Key(c.QuestionMessage.Question[0], common.GetEDNSClientSubnetIP(&c.QuestionMessage)), c.ResponseMessage)
+		c.Cache.InsertMessage(cache.Key(c.QuestionMessage.Question[0], common.GetEDNSClientSubnetIP(c.QuestionMessage)), c.ResponseMessage)
 	}
 }

core/outbound/clientbundle.go

@@ -13,7 +13,7 @@ import (
 
 type ClientBundle struct {
 	ResponseMessage *dns.Msg
-	QuestionMessage dns.Msg
+	QuestionMessage *dns.Msg
 
 	ClientList []*Client
 
@@ -26,11 +26,11 @@ type ClientBundle struct {
 
 func NewClientBundle(q *dns.Msg, ul []*common.DNSUpstream, ip string, h *hosts.Hosts, cache *cache.Cache) *ClientBundle {
 
-	cb := &ClientBundle{QuestionMessage: *q, DNSUpstreamList: ul, InboundIP: ip, Hosts: h, Cache: cache}
+	cb := &ClientBundle{QuestionMessage: q.Copy(), DNSUpstreamList: ul, InboundIP: ip, Hosts: h, Cache: cache}
 
 	for _, u := range ul {
 
-		c := NewClient(&cb.QuestionMessage, u, cb.InboundIP, cb.Hosts, cb.Cache)
+		c := NewClient(cb.QuestionMessage, u, cb.InboundIP, cb.Hosts, cb.Cache)
 		cb.ClientList = append(cb.ClientList, c)
 	}
 
@@ -52,21 +52,14 @@ func (cb *ClientBundle) ExchangeFromRemote(isCache bool, isLog bool) {
 
 	for i := 0; i < len(cb.ClientList); i++ {
 		if c := <-ch; c.ResponseMessage != nil {
-			if common.IsAnswerEmpty(c.ResponseMessage) {
-				ec = c
+			ec = c
+			if common.HasAnswer(c.ResponseMessage) {
 				break
 			}
-			cb.ResponseMessage = c.ResponseMessage
-			cb.QuestionMessage = c.QuestionMessage
-
-			if isCache {
-				cb.CacheResult()
-			}
-
-			return
 		}
 	}
-	if ec != nil {
+
+	if ec != nil && ec.ResponseMessage != nil {
 		cb.ResponseMessage = ec.ResponseMessage
 		cb.QuestionMessage = ec.QuestionMessage
 
@@ -91,6 +84,6 @@ func (cb *ClientBundle) ExchangeFromLocal() bool {
 func (cb *ClientBundle) CacheResult() {
 
 	if cb.Cache != nil {
-		cb.Cache.InsertMessage(cache.Key(cb.QuestionMessage.Question[0], common.GetEDNSClientSubnetIP(&cb.QuestionMessage)), cb.ResponseMessage)
+		cb.Cache.InsertMessage(cache.Key(cb.QuestionMessage.Question[0], common.GetEDNSClientSubnetIP(cb.QuestionMessage)), cb.ResponseMessage)
 	}
 }

core/outbound/dispatcher.go

@@ -7,9 +7,14 @@ import (
 	log "github.com/Sirupsen/logrus"
 	"github.com/miekg/dns"
 	"github.com/shawn1m/overture/core/common"
+
+	"github.com/shawn1m/overture/core/cache"
+	"github.com/shawn1m/overture/core/hosts"
 )
 
 type Dispatcher struct {
+	QuestionMessage *dns.Msg
+
 	PrimaryDNS     []*common.DNSUpstream
 	AlternativeDNS []*common.DNSUpstream
 	OnlyPrimaryDNS bool
@@ -21,10 +26,18 @@ type Dispatcher struct {
 	IPNetworkList      []*net.IPNet
 	DomainList         []string
 	RedirectIPv6Record bool
+
+	InboundIP string
+
+	Hosts *hosts.Hosts
+	Cache *cache.Cache
 }
 
 func (d *Dispatcher) Exchange() {
 
+	d.PrimaryClientBundle = NewClientBundle(d.QuestionMessage, d.PrimaryDNS, d.InboundIP, d.Hosts, d.Cache)
+	d.AlternativeClientBundle = NewClientBundle(d.QuestionMessage, d.AlternativeDNS, d.InboundIP, d.Hosts, d.Cache)
+
 	for _, cb := range [2]*ClientBundle{d.PrimaryClientBundle, d.AlternativeClientBundle} {
 		if ok := cb.ExchangeFromLocal(); ok {
 			d.ActiveClientBundle = cb
@@ -33,8 +46,8 @@ func (d *Dispatcher) Exchange() {
 	}
 
 	if d.OnlyPrimaryDNS {
-		d.PrimaryClientBundle.ExchangeFromRemote(true, true)
 		d.ActiveClientBundle = d.PrimaryClientBundle
+		d.ActiveClientBundle.ExchangeFromRemote(true, true)
 		return
 	}
 
@@ -85,9 +98,9 @@ func (d *Dispatcher) ChooseActiveClientBundle() {
 	d.ActiveClientBundle = d.PrimaryClientBundle
 	d.PrimaryClientBundle.ExchangeFromRemote(false, true)
 
-	if d.PrimaryClientBundle.ResponseMessage == nil || len(d.PrimaryClientBundle.ResponseMessage.Answer) == 0 {
-		log.Debug("Primary DNS answer is empty, finally use alternative DNS")
-		d.ActiveClientBundle = d.AlternativeClientBundle
+	if d.PrimaryClientBundle.ResponseMessage == nil || !common.HasAnswer(d.PrimaryClientBundle.ResponseMessage) {
+		//log.Debug("Primary DNS answer is empty, finally use alternative DNS")
+		//d.ActiveClientBundle = d.AlternativeClientBundle
 		return
 	}