groue / GRMustache.swift

Flexible Mustache templates for Swift
http://mustache.github.com/
MIT License
601 stars 154 forks source link

Thread Safety Improvements #36

Open groue opened 8 years ago

groue commented 8 years ago

One does not generally trigger threading issues with GRMustache:

// Thread-safe
let template = try Template(path: "/path/to/templates/document.mustache")
let rendering = try template.render(data)

But should one go down one level and expose Template Repositories to make good use of their configurability, template caching, or support for absolute paths to partials, threading issues may arise:

let templatesURL = URL(fileURLWithPath: "/path/to/templates")
let repository = TemplateRepository(baseURL: templatesURL)

// Not thread-safe (1)
repository.configuration.register(StandardLibrary.eachFilter, forKey: "each") 

// Not thread-safe (2)
let template = try repository.template(named: "document")

// Thread-safe
let rendering = try template.render(data)
  1. The configuration of a template repository is not protected against concurrent accesses: should two threads modify or read the configuration of a single repository at the same time, the program may behave in an unexpected way, or crash.

    This issue is not a big deal: template repository configuration is supposed to happen once, before any template is loaded from the repository. We can just update the documentation and add a warning.

  2. Template loading is not thread-safe: should two threads load a template at the same time, the program may behave in an unexpected way, or crash.

    Now this is a real issue, which needs to be addressed. Basically the problem lies in the fact that a template repository's cache and locked configuration are not protected against concurrent uses.

    The cache contains Mustache template ASTs, and prevents GRMustache from parsing again and again the same templates and partials. It also gives support for recursive templates. The locked configuration is a copy of the repository's configuration made as soon as the repository loads its first template. Once the configuration locked, further updates to the repository's configuration have no effect.

groue commented 8 years ago

CC @vadimeisenbergibm since this topic may be of some interest for Kitura

vadimeisenbergibm commented 7 years ago

@groue Please note that for Kitura only the following functionality is exposed in Kitura-MustacheTemplateEngine:

let template = try Template(path: filePath)
return try template.render(with: Box(context))

Since these two lines do not change GRMoustache configuration, they cannot cause concurrency issues, can they?