Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread Safety #36

Closed
jacobwilliams opened this issue Jan 8, 2015 · 5 comments
Closed

Thread Safety #36

jacobwilliams opened this issue Jan 8, 2015 · 5 comments

Comments

@jacobwilliams
Copy link
Owner

Currently the module is not thread safe (due to the global variables used when parsing a file and to manage the error conditions). Consider refactoring the code to make it thread safe.

@szaghi
Copy link

szaghi commented Jan 8, 2015

To this aim, it can be convenient to pack global vars referred to single parsed file into a derived type allowing each thread to have its own global vars set. I have successful used this approach into Lib_VTK_IO.

See you soon.

@jacobwilliams
Copy link
Owner Author

I have been thinking about this a little bit. There's probably no way to make this happen without breaking backward compatibility, so here are some thoughts on a possible implementation that has a somewhat similar usage (this could be the 4.0 release):

  • Currently, there are two major modes of using the module: (1) using json_file objects to read JSON files and (2) directly manipulating json_value pointers to build JSON structures from scratch (and also for other applications).
  • The json_file method is already pretty object-oriented, and could easily be made thread safe by moving the global variables into the type. However, this would break the json_value method, since this uses only the low-level subroutines (such as json_add, etc.). Note that json_file operations are merely wrappers to the low-level routines.
  • We could create a new class (say, json_core) which encapsulates all the subroutines (json_add, json_update, etc...). This type would contain all the variables that are currently global. The json_file class would also contain an instance of this type, but note that if you only need the things you can do with a json_file then you never need to worry about the json_core object. Here is an example:

Current usage:

    program test
     use json_module
     implicit none
     type(json_value),pointer :: p
     call json_initialize()         !initialize the module
     call json_create_object(p,'')  !create the root
     call json_add(p,'year',1805)   !add some data
     call json_add(p,'value',1.0d0) !add some data
     call json_print(p,'test.json') !write it to a file
     call json_destroy(p)           !cleanup
    end program test

Proposed usage:

    program test
     use json_module
     implicit none
     type(json_core) :: json     !<--have to declare this
     type(json_value),pointer :: p
     call json%initialize()         !initialize this instance
     call json%create_object(p,'')  !create the root
     call json%add(p,'year',1805)   !add some data
     call json%add(p,'value',1.0d0) !add some data
     call json%print(p,'test.json') !write it to a file
     call json%destroy(p)           !cleanup
    end program test

It has the advantage of being fairly easy to update your code (declare a json_core variable and then change a bunch of _ to %). You can declare as many json_core instances as you want, and they won't interfere with each other.

Note that json_file calls would remain mostly the same as they are now (it only changes under the hood). The global json_failed() routine would be removed, so errors would be checked via something like json_file%failed().

Is this clunky though? Is there a better way?

@jacobwilliams
Copy link
Owner Author

I'm going to start implementing this after #170 is finished.

@jacobwilliams
Copy link
Owner Author

This is coming along nicely on the thread-safe branch.

@jacobwilliams
Copy link
Owner Author

Done.

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

No branches or pull requests

2 participants