rt2zz / redux-persist

persist and rehydrate a redux store
MIT License
12.97k stars 867 forks source link

race condition on pause #452

Open Morriz opened 7 years ago

Morriz commented 7 years ago

Hi, in createPersistor there is an interval loop in which the keys get persisted, but they don't take into account the paused flag, leading to race conditions.

Expected behaviour:

So I propose to check for the pause flag just before the interval run and bail on pause. The operation needs to preserve atomicity, and therefore I suggest changing this part:

    // time iterator (read: debounce)
    if (timeIterator === null) {
      timeIterator = setInterval(() => {
        if (storesToProcess.length === 0) {

to:

    const len = storesToProcess.length
    // time iterator (read: debounce)
    if (timeIterator === null) {
      timeIterator = setInterval(() => {
        if ((paused && len === storesToProcess.length) || storesToProcess.length === 0) {

I will create a pull request for this.

Morriz commented 7 years ago

I just created pull request #454.

rt2zz commented 7 years ago

can you explain your use case for calling pause? in v5 there is currently no pause method, and depending on the use cases I am trying to figure out what the best api will be.

qborreda commented 7 years ago

I have one, if you let me. We have a dashboard with it's own authentication and the persisted auth reducer has all the info on the logged user, including a jwt token for the API calls. But we should let our admin clients from the main application to log into the dashboard with a jwt token written in a cookie (domain wise)

I'm having problems when the cookie is found, as the REHYDRATE event is fired after I send a logWithToken action with the cookie value ..

If the persistor had a pause/cancel rehydrate method, it would be awesome in this case ..

Morriz commented 7 years ago

I fixed it in my branch (v4) with the proposed solution. You can try installing from morriz/redux-persist and see if it works. If you use yarn you have to do an extra npm install morriz/redux-persist to make it build.

So yes, we should be able to stop persisting state, as in case of errors: we would like to be able to erase state from storage and not have to worry about the lib storing it back.

qborreda commented 7 years ago

A different approach is to write conditions on the different reducers where the REHYDRATE is going to change state, when needed, but it's very manual ..

export default function reducer(state = initialState.auth, action) {
  switch (action.type) {
    case REHYDRATE:
      // conditional check if you need it
      return action.payload.auth ? { ...state, ...action.payload.auth } : state;
...
Morriz commented 7 years ago

Too manual. We need to be able to say STOP