From 4bca452afe2b91dd6933738a87247eab960c2959 Mon Sep 17 00:00:00 2001 From: xengineering Date: Mon, 3 Jul 2023 13:55:52 +0200 Subject: BROKEN: Make bug reliable This should make the race appear reliable if a second connection attempt is made. It is important to have a reliable error to fix it reliably. --- xmpp/stream_pair.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/xmpp/stream_pair.go b/xmpp/stream_pair.go index 87df86a..2feea6b 100644 --- a/xmpp/stream_pair.go +++ b/xmpp/stream_pair.go @@ -13,13 +13,13 @@ func runStreamPair(s *session) { for { select { - case data := <-s.in: - switch data.(type) { - case SessionShouldDisconnect: - return - default: - log.Printf("Unknown data '%d'!\n", data) - } +// case data := <-s.in: +// switch data.(type) { +// case SessionShouldDisconnect: +// return +// default: +// log.Printf("Unknown data '%d'!\n", data) +// } case t := <-s.rx: err := buf.add(t) if err != nil { @@ -66,13 +66,13 @@ func openStream(s *session) xml.EndElement { func syncStreams(s *session) { for { select { - case data := <-s.in: - switch data.(type) { - case SessionShouldDisconnect: - return - default: - log.Printf("Unhandled data '%d' during stream sync!\n", data) - } +// case data := <-s.in: +// switch data.(type) { +// case SessionShouldDisconnect: +// return +// default: +// log.Printf("Unhandled data '%d' during stream sync!\n", data) +// } case t := <-s.rx: switch token := t.(type) { case xml.StartElement: -- cgit v1.2.3-70-g09d2 From 938e92387ae1aa3f771ed219ee65b2dbc34b6738 Mon Sep 17 00:00:00 2001 From: xengineering Date: Mon, 3 Jul 2023 13:58:58 +0200 Subject: Fix race condition on struct Limox To make message passing to the XMPP session channel non-blocking short living Goroutines are used. Before this commit they were executing closures which capture l as xengineering.eu/limox.Limox pointer and used it to get the channel where the message should be passed to. While channels are safe with respect to race conditions, structs are not. Thus the member access of the Limox struct was a race condition in certain side cases like bad network connection where those Goroutines lived for a longer time. The solution is to make a copy of the l.sessionIn channel and use this copy in the closures. This is valid since channels do not need pointers to them and furthermore are thread safe. --- limox.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/limox.go b/limox.go index 87e20de..5bb2d57 100644 --- a/limox.go +++ b/limox.go @@ -80,6 +80,7 @@ func (l *Limox) run() error { } func (l *Limox) buttonCallback() { + c := l.sessionIn switch l.State { case Disconnected: jid := l.JidEditor.Text() @@ -89,10 +90,10 @@ func (l *Limox) buttonCallback() { l.sessionIn = xmpp.StartSession(l.sessionOut, jid, pwd) l.State = Connecting case Connecting: - go func() { l.sessionIn <- xmpp.SessionShouldDisconnect{} }() + go func() { c <- xmpp.SessionShouldDisconnect{} }() l.State = Disconnected case Connected: - go func() { l.sessionIn <- xmpp.SessionShouldDisconnect{} }() + go func() { c <- xmpp.SessionShouldDisconnect{} }() l.State = Disconnected } } -- cgit v1.2.3-70-g09d2 From 58aecc9d860a18767aa51d0d94793057e1aa57e2 Mon Sep 17 00:00:00 2001 From: xengineering Date: Mon, 3 Jul 2023 14:03:19 +0200 Subject: Remove intended breakage This removes the injected broken code which made the race condition reproducible. --- xmpp/stream_pair.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/xmpp/stream_pair.go b/xmpp/stream_pair.go index 2feea6b..87df86a 100644 --- a/xmpp/stream_pair.go +++ b/xmpp/stream_pair.go @@ -13,13 +13,13 @@ func runStreamPair(s *session) { for { select { -// case data := <-s.in: -// switch data.(type) { -// case SessionShouldDisconnect: -// return -// default: -// log.Printf("Unknown data '%d'!\n", data) -// } + case data := <-s.in: + switch data.(type) { + case SessionShouldDisconnect: + return + default: + log.Printf("Unknown data '%d'!\n", data) + } case t := <-s.rx: err := buf.add(t) if err != nil { @@ -66,13 +66,13 @@ func openStream(s *session) xml.EndElement { func syncStreams(s *session) { for { select { -// case data := <-s.in: -// switch data.(type) { -// case SessionShouldDisconnect: -// return -// default: -// log.Printf("Unhandled data '%d' during stream sync!\n", data) -// } + case data := <-s.in: + switch data.(type) { + case SessionShouldDisconnect: + return + default: + log.Printf("Unhandled data '%d' during stream sync!\n", data) + } case t := <-s.rx: switch token := t.(type) { case xml.StartElement: -- cgit v1.2.3-70-g09d2