Skip to content
This repository was archived by the owner on Dec 15, 2025. It is now read-only.
This repository was archived by the owner on Dec 15, 2025. It is now read-only.

Global mutable state is not a great idea #265

Closed

Description

@ash2k

Suppose I have a program that uses two packages: "github.com/user1/a" ("a" below) and "github.com/user2/b" ("b" below). "a" uses json-iterator library with a custom Type Decoder. It calls jsoniter.RegisterTypeDecoderFunc() in some init() method somewhere to add a decoder. Package "b" uses "encoding/json" from standard library. Everything works.
One day author of "b" finds out about great performant library json-iterator and changes the package to use it. All tests pass, performance is great, everybody happy, right?
Next day I decide to upgrade package "b" to a new version that is much more performant. But my tests fail in a weird way. What is going on?
So it turns out that both packages want to override how e.g. interface {} is handled and both call jsoniter.RegisterTypeDecoderFunc("interface {}", myFunc). Of course each one of them does something different and the last one wins - either "a" or "b" depending on the order compiler resolves imports.

As an author of packages "a" or "b" I have no way to protect my users from such issues. As a consumer of "a" or "b" I have no way to resolve the conflict. Would be great to do something about it. For example make decoders and other global mutable things part of the Config struct so that BOTH "a" and "b" can have their own local configurations, completely isolated from the outside world.
Thank you.

Activity

  1. taowen commented on Apr 20, 2018

    @taowen
    Contributor

    RegisterTypeDecoderFunc should only be used when you want to have global config, and it is setup in init time.
    For isolated customization, use https://github.com/json-iterator/go/blob/master/config.go jsoniter.Config and RegisterExtension on the config object.

  2. ash2k commented on Apr 21, 2018

    @ash2k
    ContributorAuthor

    I overlooked this method, thanks. This is better than nothing.
    However I still see issues with it:

    1. This method is defined on frozenConfig type. And hence package "a" can mutate, say, jsoniter.ConfigCompatibleWithStandardLibrary. That will disrupt all the users of that global variable (that should be immutable/"frozen") in the same program. I think this method should be on Config. Or maybe not a method but just a field.
    2. Ok, lets say I have created an isolated configuration.
      var myOwnPrivate = jsoniter.Config{}.Froze(). This actually does not completely isolate my package "a" from package "b" that may decide to call the global jsoniter.RegisterTypeDecoderFunc(). _getTypeDecoderFromExtension() checks the global list of extensions first, and then the registered ones in a private configuration.
      https://github.com/json-iterator/go/blob/master/reflect_extension.go#L256-L261

    WDYT? Do you agree this is an issue worth fixing?

  3. taowen commented on Apr 22, 2018

    @taowen
    Contributor

    The api is designed to favor convenience. I understand your concern, but forcing isolation will complicate the api.

  4. added a commit that references this issue on Jun 21, 2018
  5. foamzou commented on Jul 18, 2024

    @foamzou

    Agree with @ash2k . We cannot prevent the libraries we depend on from using global configurations at some point, which would put our service in an insecure situation and result in unexpected behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions