swoole / swoole-src

šŸš€ Coroutine-based concurrency library for PHP
https://www.swoole.com
Apache License 2.0
18.27k stars 3.16k forks source link

Table object does not work across threads #2067

Closed ClosetGeek-Git closed 4 years ago

ClosetGeek-Git commented 5 years ago

I can confirm that table objects do not work across threads. Documents should be updated to clarify.

As stated in https://www.swoole.co.uk/docs/modules/swoole-table

Why to use swoole table

Code I used

<?php

class A extends Thread {
    public $table;
    public function __construct($table) {
        $this->table = $table;
    }
    public function run() {
        if (!$this->table->set('test_key', array('id' => 1))) {
            echo "could not set\n";
        }
    }
}

class B extends Thread {
    public $table;
    public function __construct($table) {
        $this->table = $table;
    }
    public function run() {
        $ret = $this->table->get('test_key');
        if (!($ret and is_array($ret) and $ret['id'] == 1)) {
            echo "some other error\n";
        }
    }
}

$table = new swoole_table(65536);
$table->column('id', swoole_table::TYPE_INT);
$table->create();

$a = new A($table);
$a->start();

$b = new B($table);
$b->start();

$b->join();

expected results: get 'test_key' from table

actual results:

PHP Fatal error:  Uncaught Error: Call to a member function set() on null in /home/jason/chan_thread.php:9
Stack trace:
#0 [internal function]: A->run()
#1 {main}
  thrown in /home/jason/chan_thread.php on line 9
PHP Fatal error:  Uncaught Error: Call to a member function get() on null in /home/jason/chan_thread.php:22
Stack trace:
#0 [internal function]: B->run()
#1 {main}
  thrown in /home/jason/chan_thread.php on line 22
ghost commented 5 years ago

Thats not the way table works, i guess ?

https://github.com/swoole/swoole-src/issues/2034#issuecomment-429503111

But, i would like to have some additional explanation about this, too :blush:

twose commented 5 years ago

Don't use multithreading in swoole, we have coroutine.

ClosetGeek-Git commented 5 years ago

@twose you should change the documentation. I understand that a possible language barrier exists but saying that an userspace extension is safe to use across threads then saying "don't use multithreading" is confusing. Are the docs referring to a internal/native api within swoole?

ClosetGeek-Git commented 5 years ago

Just for clarity; are table objects thread safe internally within native extensions. If I spawn a native thread within a different extension can I safely access an existing swoole table object within the new thread?

for example

// brief look seems possible to include table header for base table functions
#include "include/table.h"

void *do_work(void *swoole_table_ptr)
{
    char *key = "test_key";
    zend_size_t = 8;

    swTableRow *_rowlock = NULL;
    swTable *table = swoole_get_object((zval*)swoole_table_pt);

    if (!table->memory)
    {
        RETURN_FALSE;
    }

    swTableRow *row = swTableRow_get(table, key, keylen, &_rowlock);
    ... get data from row
    ... etc 
    ... etc
}

PHP_METHOD(SomeClass, __construct) 
{
    zval *swoole_table_obj;

    if (zend_parse_parameters(ZEND_NUM_ARGS(), "o", &swoole_table_obj, ...) != SUCCESS) {
        return;
    }

    pthread_t work_thread;
    pthread_create(&work_thread, NULL, do_work, &swoole_table_obj);
}
ClosetGeek-Git commented 5 years ago

Even better (zval itself is not thread friendly)

// brief look seems possible to include table header for base table functions
#include "include/table.h"

void *do_work(void *table_ptr)
{
    char *key = "test_key";
    zend_size_t = 8;
    swTableRow *_rowlock = NULL;

    swTableRow *row = swTableRow_get((swTable *)table_ptr, key, keylen, &_rowlock);
    ... get data from row
    ... etc 
    ... etc
}

PHP_METHOD(SomeClass, __construct) 
{
    zval *swoole_table_obj;

    if (zend_parse_parameters(ZEND_NUM_ARGS(), "o", &swoole_table_obj, ...) != SUCCESS) {
        return;
    }

    swTable *table = swoole_get_object(swoole_table_obj);

    pthread_t work_thread;
    pthread_create(&work_thread, NULL, do_work, &table);
}
matyhtf commented 5 years ago

The swoole_table is not thread-safety. No compatibility with pthreads has been considered since the design

ClosetGeek-Git commented 5 years ago

Then I guess that settles it - no userspace thread support and no native thread support sums up to no thread support at all and therefore such statements should be completely removed from the docs. I for one spent hours investigating this because the docs clearly (yet incorrectly) state that swoole::table is safe across multiple threads...

ghost commented 5 years ago

You should read the complete documentation, start here: https://wiki.swoole.com/wiki/index/prid-1

For me they do a very good job. šŸ„‡

It's not easy to code this level, to help answering issues daily and write documentation as soon as possible :dizzy_face:

šŸ’Æ points where we stay šŸ˜‰

swoole rocks :rocket:

yellow1912 commented 5 years ago

I think all this comes from the confusion of swoole built in threads (they have that concept too, look at the source code) and the php7 threads?

Perhaps the document should be updated to clarify this. I have a hunch that with coroutine support there is no gain with supporting the php7 threads at the same time. Perhaps I'm wrong.

ClosetGeek-Git commented 5 years ago

A coroutine should not be (and generally is not) called a thread in today's systems. In coroutine's hayday systems only had one processor and one core and the only concept of a thread was a time slice. In today's systems - and in the industry in general - a thread is understood as a truly concurrent, i.e. not shared, slice of time. calling a coroutine a thread when the underlying hardware, OS, native api, and userspace apis each support true threads (and each use this definition as a standard) is confusing to say the least. Time is money, and in a professional setting it also risks ones reputation. I'm glad I was on my own time while weeding through this.

ghost commented 5 years ago

In coroutine's hayday systems only had one processor and one core and the only concept of a thread was a time slice.

In today's systems - and in the industry in general - a thread is understood as a truly concurrent, i.e. not shared, slice of time.

You are absolutely wrong. You should learn more (or something) about systems programming.

This has nothing to do with as how is something understood or is somewhere called, they all are technical constructs with several differences in use of ressources, of sharing memory, of constructing, destructing, pausing, resuming, blocking, sheduling and so on.

Swoole is going the right way using coroutines, as we see, today, fibers and coroutines are mostly used in high-concurrency systems. But they never have called their coroutines threads as you understands threads. Is this your understanding?

As said, you should read the complete documentation. Start at reactors.

I really do not understand why you were working in ZTS, use Posix threads, want to share the swoole table there and now battling against the documentation?!

In today's systems - and in the industry in general - a thread is understood as a truly concurrent, i.e. not shared, slice of time.

no words.

and in a professional setting it also risks ones reputation. I'm glad I was on my own time while weeding through this.

for sure, maybe some guiding would help you

the only concept of a thread was a time slice

waaaaa???

ClosetGeek-Git commented 5 years ago

@flddr Before you talk about ones "systems programming" capabilities you should probably know that coroutines are not and will never be a construct. This is not a matter of "look and feel" or other trivial concepts of how a thing can be represented through the lens of a specific language or platform. coroutines are specifically when one set of instructions - (think asm, not PHP) - yield to another. This is very beneficial. This is NOT comparable to swooles concept of "coroutines" where non-blocking network code yields although it may be similar on a higher level. Don't mention reactors, they have nothing to do with this issue other than how swoole has semantically broken down non-blocking network code to look-and-feel similar to coroutines. Swoole does not offer true corotines as they advertise, and their objects ARE NOT thread safe (in userspace or native) as their documentation clearly states. If you do not understand this I am sorry - FWI you are specifically one of the people who has previously been confused by this misleading concept of "thread safe"

ClosetGeek-Git commented 5 years ago

I want to add that swoole IS doing an awesome job and that they DO out perform competitors. additionally, maybe even more importantly, they are one of the most successful software products coming out of China on github - and on a technical level they can even reshape PHP services. That said, the way they state things differently.

ClosetGeek-Git commented 5 years ago

Before you argue keep in mind that each PHP functions (think basic fopen) requires hundreds and even THOUSANDS of instructions each once you start picking through all the glue. Tell me how swoole has broken this down into true corotines. I would like links to code within this repository. Including their reactor as you mention.

ClosetGeek-Git commented 5 years ago

Moral of the story - this documentation and how they repent themselves is sh*t but the project is awesome.

ghost commented 5 years ago

you should probably know that coroutines are not and will never be a construct.

? They are and will ever be (for the next decades)

Before you argue keep in mind that each PHP functions (think basic fopen) requires hundreds and even THOUSANDS of instructions each once you start picking through all the glue. Tell me how swoole has broken this down into true corotines.

what? (FYI: fopen simply is a wrapper around C fopen from stdio.h) (as in nearly every language and systems that runs on *nix or posix)

This is NOT comparable to swooles concept of "coroutines" where non-blocking network code yields although it may be similar on a higher level.

FWI you are specifically one of the people who has previously been confused by this misleading concept of "thread safe"

?

Swoole does not offer true corotines as they advertise, and their objects ARE NOT thread safe (in userspace or native) as their documentation clearly states.

Swoole coroutines and coroutines in general have nothing to do with the PHP Threadsupport (ZTS) you are talking about. Really nothing. There is no commonality.

Moral of the story - this documentation and how they repent themselves is sh*t but the project is awesome.

šŸ‘Ž

I am sorry that you misunderstood all of this. Sometimes, a little break helps šŸ˜Š