[FIXED] Exception with persistent connections: "Cannot unset string offsets" about net_routeros HOT 11 CLOSED

Ok, so just to report back, the fix to Cache_SHM seems to fix everything regarding persistent connections. The reason why i needed this so badly is because im working on a poller in order to display realtime data as you can see here (the gauges update twice a second):

I left it running for like 12 hours and the connection remained open, without issues so far.
Im closing this as the problem is gone now.
Thanks a lot for this great library!
// Diego

from net_routeros.

I'm very happy to finally see a success story with persistent connections 😁 .

from net_routeros.

After some more investigation, the problem seems to be with the APCu adapter in the Cache_SHM library...

They have these 2 methods defined there:

    /**
     * Obtains a named lock.
     *
     * @param string $key     Name of the key to obtain. Note that $key may
     *     repeat for each distinct $persistentId.
     * @param double $timeout If the lock can't be immediately obtained, the
     *     script will block for at most the specified amount of seconds.
     *     Setting this to 0 makes lock obtaining non blocking, and setting it
     *     to NULL makes it block without a time limit.
     *
     * @return bool TRUE on success, FALSE on failure.
     */
    public function lock($key, $timeout = null)
    {
        $lock = $this->persistentId . 'l ' . $key;
        $hasTimeout = $timeout !== null;
        $start = microtime(true);
        while (!apcu_add($lock, 1)) {
            if ($hasTimeout && (microtime(true) - $start) > $timeout) {
                return false;
            }
        }
        static::$locksBackup[$this->persistentId] = $key;
        return true;
    }
    /**
     * Releases a named lock.
     *
     * @param string $key Name of the key to release. Note that $key may
     *     repeat for each distinct $persistentId.
     *
     * @return bool TRUE on success, FALSE on failure.
     */
    public function unlock($key)
    {
        $lock = $this->persistentId . 'l ' . $key;
        $success = apcu_delete($lock);
        if ($success) {
            unset(
                static::$locksBackup[$this->persistentId][array_search(
                    $key,
                    static::$locksBackup[$this->persistentId],
                    true
                )]
            );
            return true;
        }
        return false;
    }

Ant the problem is specifically with the array_search() implementation inside the unlock method, because static::$locksBackup[$this->persistentId] will NEVER return an array, causing a warning coming from array_search() complaining about parameter 2 not being an array, and later an exception from the call to unset() trying to unset a string....

I am not really sure how can this be so wrong, as I assume that this library is being used by lots of people (?)... or maybe im just missing something really obvious, but rewriting the unlock() method like this:

  public function unlock($key)
  {
    $lock = $this->persistentId . 'l ' . $key;
    $success = apcu_delete($lock);
    if ($success) {
      unset(
        static::$locksBackup[array_search(
          $key,
          static::$locksBackup,
          true
        )]
      );
      return true;
    }
    return false;
  }

fixes all the problems and persistent connections work as expected.

The Cache_SHM library does not seem to be actively maintained tho, so i don't really know. What is even more strange is that all of the adapters have the same problem on the unlock() method, so i dont really know how can that code actually work at all without my previous mod...

Comments ?

// Diego

from net_routeros.

And even more strange, on the APCu adapter, inside the construct method, they initialize static::$locksBackup[$this->persistentId] like this:

static::$locksBackup[$this->persistentId] = array();

which would make sense, but then later on the lock() method they set static::$locksBackup[$this->persistentId] to be a string, this way:

static::$locksBackup[$this->persistentId] = $key;

and from the documentation on that very same method: @param string $key.... i am lost at this point as to how could this SHM library work at all.
Im in a deep WTF momentus now, trying to assume that im missing something very obvious, and there is no way this code could be so wrong.

// Diego

from net_routeros.

am not really sure how can this be so wrong, as I assume that this library is being used by lots of people (?)

Sadly no. Few people use persistent connections with PEAR2_Net_RouterOS (as opposed to the many that use non-persistent connections), and equally few use Cache_SHM on its own (as opposed to using APCu directly).

The Cache_SHM library does not seem to be actively maintained tho, so i don't really know. What is even more strange is that all of the adapters have the same problem on the unlock() method, so i dont really know how can that code actually work at all without my previous mod...

It is maintained. It's just that I can't really unit test it in any way due to its specific nature, which, combined with the above, makes it impossible for me to really detect and fix such issues early.

Comments ?

The idea of having the static::$locksBackup[$this->persistentId] is to namespace the locks and data based on the ID provided at construction time.

With that in mind, the correct fix is to change

static::$locksBackup[$this->persistentId] = $key;

to

static::$locksBackup[$this->persistentId][] = $key;

and leave unlock() unmodified.

from net_routeros.

Even if you change that, for other people using that library, the code will break if you call unlock() with a string as they suggest on the inline docs... that was my whole point in here heh, but yeah, your fix is what i was actually thinking of in order to keep the SHM lib untouched, of maybe just fork it myself...

from net_routeros.

@boenrobot are you the maintainer for the SHM lib ?

from net_routeros.

Even if you change that, for other people using that library, the code will break if you call unlock() with a string as they suggest on the inline docs...

Until I make a new release with this fix, sure... But hold your horses. I only found out about it right now, from you.

@boenrobot are you the maintainer for the SHM lib ?

Yes.

from net_routeros.

Ohhh! in that case i will just send you a PR if you want, thought that the SHM lib was kind of abandonware, and somehow i did not see it listed on your GH profile.
Its very good news then if you are the guy behind that too! :)

from net_routeros.

Sure. Go for it.

from net_routeros.

PR is ready: Here we go: pear2/Cache_SHM#1

Thank you!

from net_routeros.