OrgWiki/go-org-orgwiki
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix race condition surrounding global orgWriter

Browse source 
Since writers are normally only used synchronously (i.e. to write one document
at a time), we don't guard modifications to their internal
state (e.g. temporarily replacing the string.Builder in WriteNodesAsString)
against race conditions.

The package global `orgWriter` and corresponding use cases of it (`org.String`,
`$node.String`) break that pattern - the writer is potentially used from
multiple go routines at the same time. This results in race conditions that
manifest as error messages like e.g.

    could not write output: runtime error: invalid memory address or nil pointer dereference. Using unrendered content.

Additionally, since we catch panics in `Document.Write`, the corresponding
stack trace is lost and dependents of go-org never know what hit them.

As using a writer across simultaneously across go routines is not a standard
pattern, we'll sync the use of the global `orgWriter` instead of trying to make
the actual writer threadsafe; less code noise for the common use case.
This commit is contained in:
Niklas Fasching 2023-03-12 11:08:19 +01:00
parent 18314a9f41
commit 5464ab37d2
12 changed files with 41 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

8
org/block.go
View file

@ -177,7 +177,7 @@ func (b Block) ParameterMap() map[string]string {
return m
}
func (n Example) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Block) String() string { return orgWriter.WriteNodesAsString(n) }
func (n LatexBlock) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Result) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Example) String() string { return String(n) }
func (n Block) String() string { return String(n) }
func (n LatexBlock) String() string { return String(n) }
func (n Result) String() string { return String(n) }

8
org/document.go
View file

@ -20,6 +20,7 @@ import (
"log"
"os"
"strings"
"sync"
)
type Configuration struct {
@ -77,6 +78,7 @@ var lexFns = []lexFn{
}
var nilToken = token{"nil", -1, "", nil}
var orgWriterMutex = sync.Mutex{}
var orgWriter = NewOrgWriter()
// New returns a new Configuration with (hopefully) sane defaults.
@ -95,7 +97,11 @@ func New() *Configuration {
}
// String returns the pretty printed Org mode string for the given nodes (see OrgWriter).
func String(nodes []Node) string { return orgWriter.WriteNodesAsString(nodes...) }
func String(nodes ...Node) string {
orgWriterMutex.Lock()
defer orgWriterMutex.Unlock()
return orgWriter.WriteNodesAsString(nodes...)
}
// Write is called after with an instance of the Writer interface to export a parsed Document into another format.
func (d *Document) Write(w Writer) (out string, err error) {

4
org/drawer.go
View file

@ -93,5 +93,5 @@ func (d *PropertyDrawer) Get(key string) (string, bool) {
return "", false
}
func (n Drawer) String() string { return orgWriter.WriteNodesAsString(n) }
func (n PropertyDrawer) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Drawer) String() string { return String(n) }
func (n PropertyDrawer) String() string { return String(n) }

2
org/footnote.go
View file

@ -32,4 +32,4 @@ func (d *Document) parseFootnoteDefinition(i int, parentStop stopFn) (int, Node)
return consumed, definition
}
func (n FootnoteDefinition) String() string { return orgWriter.WriteNodesAsString(n) }
func (n FootnoteDefinition) String() string { return String(n) }

2
org/headline.go
View file

@ -126,4 +126,4 @@ func (parent *Section) add(current *Section) {
}
}
func (n Headline) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Headline) String() string { return String(n) }

4
org/html_writer.go
View file

@ -409,14 +409,14 @@ func (w *HTMLWriter) WriteRegularLink(l RegularLink) {
if l.Description == nil {
w.WriteString(fmt.Sprintf(`<img src="%s" alt="%s" title="%s" />`, url, url, url))
} else {
description := strings.TrimPrefix(String(l.Description), "file:")
description := strings.TrimPrefix(String(l.Description...), "file:")
w.WriteString(fmt.Sprintf(`<a href="%s"><img src="%s" alt="%s" /></a>`, url, description, description))
}
case "video":
if l.Description == nil {
w.WriteString(fmt.Sprintf(`<video src="%s" title="%s">%s</video>`, url, url, url))
} else {
description := strings.TrimPrefix(String(l.Description), "file:")
description := strings.TrimPrefix(String(l.Description...), "file:")
w.WriteString(fmt.Sprintf(`<a href="%s"><video src="%s" title="%s"></video></a>`, url, description, description))
}
default:

24
org/inline.go
View file

@ -400,7 +400,7 @@ func isValidPostChar(r rune) bool {
func isValidBorderChar(r rune) bool { return !unicode.IsSpace(r) }
func (l RegularLink) Kind() string {
description := String(l.Description)
description := String(l.Description...)
descProtocol, descExt := strings.SplitN(description, ":", 2)[0], path.Ext(description)
if ok := descProtocol == "file" || descProtocol == "http" || descProtocol == "https"; ok && imageExtensionRegexp.MatchString(descExt) {
return "image"
@ -420,14 +420,14 @@ func (l RegularLink) Kind() string {
return "regular"
}
func (n Text) String() string { return orgWriter.WriteNodesAsString(n) }
func (n LineBreak) String() string { return orgWriter.WriteNodesAsString(n) }
func (n ExplicitLineBreak) String() string { return orgWriter.WriteNodesAsString(n) }
func (n StatisticToken) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Emphasis) String() string { return orgWriter.WriteNodesAsString(n) }
func (n InlineBlock) String() string { return orgWriter.WriteNodesAsString(n) }
func (n LatexFragment) String() string { return orgWriter.WriteNodesAsString(n) }
func (n FootnoteLink) String() string { return orgWriter.WriteNodesAsString(n) }
func (n RegularLink) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Macro) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Timestamp) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Text) String() string { return String(n) }
func (n LineBreak) String() string { return String(n) }
func (n ExplicitLineBreak) String() string { return String(n) }
func (n StatisticToken) String() string { return String(n) }
func (n Emphasis) String() string { return String(n) }
func (n InlineBlock) String() string { return String(n) }
func (n LatexFragment) String() string { return String(n) }
func (n FootnoteLink) String() string { return String(n) }
func (n RegularLink) String() string { return String(n) }
func (n Macro) String() string { return String(n) }
func (n Timestamp) String() string { return String(n) }

10
org/keyword.go
View file

@ -187,8 +187,8 @@ func (d *Document) loadSetupFile(k Keyword) (int, Node) {
return 1, k
}
func (n Comment) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Keyword) String() string { return orgWriter.WriteNodesAsString(n) }
func (n NodeWithMeta) String() string { return orgWriter.WriteNodesAsString(n) }
func (n NodeWithName) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Include) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Comment) String() string { return String(n) }
func (n Keyword) String() string { return String(n) }
func (n NodeWithMeta) String() string { return String(n) }
func (n NodeWithName) String() string { return String(n) }
func (n Include) String() string { return String(n) }

6
org/list.go
View file

@ -118,6 +118,6 @@ func (d *Document) parseListItem(l List, i int, parentStop stopFn) (int, Node) {
return i - start, ListItem{bullet, status, value, nodes}
}
func (n List) String() string { return orgWriter.WriteNodesAsString(n) }
func (n ListItem) String() string { return orgWriter.WriteNodesAsString(n) }
func (n DescriptiveListItem) String() string { return orgWriter.WriteNodesAsString(n) }
func (n List) String() string { return String(n) }
func (n ListItem) String() string { return String(n) }
func (n DescriptiveListItem) String() string { return String(n) }

4
org/paragraph.go
View file

@ -43,5 +43,5 @@ func (d *Document) parseHorizontalRule(i int, parentStop stopFn) (int, Node) {
return 1, HorizontalRule{}
}
func (n Paragraph) String() string { return orgWriter.WriteNodesAsString(n) }
func (n HorizontalRule) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Paragraph) String() string { return String(n) }
func (n HorizontalRule) String() string { return String(n) }

2
org/table.go
View file

@ -134,4 +134,4 @@ func isSpecialRow(rawColumns []string) bool {
return isAlignRow
}
func (n Table) String() string { return orgWriter.WriteNodesAsString(n) }
func (n Table) String() string { return String(n) }