copy assignment operator of 'Socket' is implicitly deleted because field 'socket' has a deleted copy assignment operator

[jeffrtc]

I've designed a thin wrapper around Seastar output stream. 

I started to get seg faults when invoking the send method from a third party non-seastar library invoked callback.

I thought it might have to do with the Wrapper object lifetime, and then I added a destructor to the wrapper to detect object going out of scope, but I can't compile the code with the destructor,

        class Socket {

            private:

                seastar::output_stream<char> socket;

            public:

                std::string peer;

            public:

                Socket() = default;

                template <typename... Args>

                explicit Socket(seastar::output_stream<char> &&s, std::string p) : socket(std::move(s)), peer(p) {}

                ~Socket() {

                    std::cout << "Socket has gone out of scope" << "\n";

                }

                seastar::future<> send(std::string message) {

                    co_await socket.write(message);

                    co_await socket.flush();

                }

                seastar::future<> close() {

                    co_await socket.close();

                }

        };

Compilation fails with,

error: object of type 'Socket' cannot be assigned because its copy assignment operator is implicitly deleted

        connection->second.socketObj = std::move(socketObj);

                                  ^

./socketwrapper.h:44:46: note: copy assignment operator of 'Socket' is implicitly deleted because field 'socket' has a deleted copy assignment operator

                seastar::output_stream<char> socket;

If I comment out destructor, the Compilation works, but I get seg faults like below,

Thread 6 "ProxyServer" received signal SIGSEGV, Segmentation fault.

[Switching to Thread 0x7fffeffff640 (LWP 2750693)]

seastar::add_to_flush_poller (os=0x6000000c5d60) at ../../src/core/reactor.cc:4189

4189        engine()._flush_batching.emplace_back(os);

(gdb) c

Continuing.

Segmentation fault.

Backtrace:

  0xea74ee

  0xe6eed0

  0xe6f64c

  0xe6f6d2

  0x7ffff734c1ef

  0xe6cf23

  0xc5d399

  0xc3eac0

  0xa9de25

  0xa9f8be

  0xdf547c

  0xdf4ee5

  0xdf4bdd

  0xdf48ff

  0xdf4772

  0xdf44fe

  0xdf34aa

  0x7ffff701a830

  0x7ffff701a74e

  0x7ffff701193c

  0x7ffff701112b

  0x7ffff7010fb2

  0x7ffff70138b8

  0x7ffff702f4ed

  0x7ffff706f339

  0x7ffff706ddd0

  0x7ffff706b9c9

  0x7ffff7069adf

  0x7ffff7065ce9

  0x7ffff706f5cf

  0x7ffff6fbbf62

  0x7ffff6fbbbf6

  0x7ffff6fa4628

  0x7ffff7021b78

  0x7ffff7088030

  0x7ffff7085c1c

  0x7ffff708e7e9

  0x7ffff708dbf2

  0x7ffff708c9da

  0x7ffff708b966

  0x7ffff708ac74

  0x7ffff708f4ab

  0x7ffff708ef71

  0x7ffff708e857

  0x7ffff708dcc8

  0x7ffff708ca9c

  0x7ffff7090c5c

  0x7ffff70909f1

  0x7ffff709073a

  0x7ffff709108b

  0x7ffff7090d1d

  0x7ffff700b039

  0x7ffff700a7c3

  0x7ffff700c22a

  0x7ffff700b876

  0x7ffff700abeb

  0x7ffff700ac22

  0x7ffff700ac37

  0x7ffff73494af

  0x7ffff7009eaf

  0x7ffff700acf8

  0x7ffff700a377

  0x7ffff70907ac

  0x7ffff705353c

  0x7ffff708cc1c

  0x7ffff708e94d

  0x7ffff6f43c01

  0x7ffff7091aa2

  0x7ffff7091a03

  0x7ffff709510c

  0x7ffff7095058

  0x7ffff7094fc2

  0x7ffff7094f50

  0x7ffff7094ef7

  0x7ffff7094ea0

  0x7ffff7094e41

  0x7ffff7094e13

  0x7ffff7094df3

  /lib/x86_64-linux-gnu/libstdc++.so.6+0xda693

  /lib/x86_64-linux-gnu/libpthread.so.0+0x944f

  /lib/x86_64-linux-gnu/libc.so.6+0x117d52

Thread 6 "ProxyServer" received signal SIGSEGV, Segmentation fault.

seastar::add_to_flush_poller (os=0x6000000c5d60) at ../../src/core/reactor.cc:4189

4189        engine()._flush_batching.emplace_back(os);

(gdb) c

Continuing.

[Thread 0x7fffe67fc640 (LWP 2750702) exited]

[Thread 0x7fffe6ffd640 (LWP 2750701) exited]

[Thread 0x7fffe77fe640 (LWP 2750700) exited]

[Thread 0x7fffe7fff640 (LWP 2750699) exited]

[Thread 0x7fffec93c640 (LWP 2750698) exited]

[Thread 0x7fffed13d640 (LWP 2750697) exited]

[Thread 0x7fffed93e640 (LWP 2750696) exited]

[Thread 0x7fffeeffd640 (LWP 2750695) exited]

[Thread 0x7fffef7fe640 (LWP 2750694) exited]

[Thread 0x7fffeffff640 (LWP 2750693) exited]

[Thread 0x7fffedbff640 (LWP 2750680) exited]

[Thread 0x7fffee1ff640 (LWP 2750679) exited]

[Thread 0x7ffff45ff640 (LWP 2750678) exited]

[Thread 0x7ffff58b7640 (LWP 2750677) exited]

[Avi Kivity]

On 16/09/2021 02.34, jeffrtc wrote:

I've designed a thin wrapper around Seastar output stream. 

I started to get seg faults when invoking the send method from a third party non-seastar library invoked callback.

I thought it might have to do with the Wrapper object lifetime, and then I added a destructor to the wrapper to detect object going out of scope, but I can't compile the code with the destructor,

<snip>

Compilation fails with,

error: object of type 'Socket' cannot be assigned because its copy assignment operator is implicitly deleted

        connection->second.socketObj = std::move(socketObj);

                                  ^

It's pretty common to have disabled copy constructors in Seastar. Streams are not copyable, what does it mean to copy a stream? Does the output of the both copies appear in the file/socket you are writing to?

You can capture a reference to the output_stream if the lifetime of the object allows it, or use lw_shared_ptr if not. Note you must have at most one pending operation on the stream, and at most one close() call in the end. So it's a lot better to have just a single owner.

btw, since you do use std::move(socketObj), perhaps the problem is that socketObj is const and so the move is defeated.

./socketwrapper.h:44:46: note: copy assignment operator of 'Socket' is implicitly deleted because field 'socket' has a deleted copy assignment operator

                seastar::output_stream<char> socket;

If I comment out destructor, the Compilation works, but I get seg faults like below,

Thread 6 "ProxyServer" received signal SIGSEGV, Segmentation fault.

[Switching to Thread 0x7fffeffff640 (LWP 2750693)]

seastar::add_to_flush_poller (os=0x6000000c5d60) at ../../src/core/reactor.cc:4189

4189        engine()._flush_batching.emplace_back(os);

(gdb) c

Continuing.

That's not surprising.

--
You received this message because you are subscribed to the Google Groups "seastar-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to seastar-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/cc68be2e-b52d-40c7-93fd-650a549e789cn%40googlegroups.com.

[jeffrtc]

I made an accident and apparently sent my response to you directly ("To author") instead everyone. I can't even see it.

Maybe you can copy it when you are responding. Let me know if you didn't receive it.  

[Avi Kivity]

output_stream is movable, but it cannot be moved once I/O has started.

Storing the socket object in the map should work, as long as it's done before I/O starts, and as long as the map does not relocate objects (std::unordered_map and std::map are fine in this regard).

Passing seastar::socket to your Socket and extracting the output_stream at that point is cleaner IMO (i.e. extract it in Socket's constructor, and use unordered_map::emplace() to make sure the Socket is constructed in place and not moved into place.

On 22/09/2021 02.45, jeffrtc wrote:

Avi,

I was able to fix the `error: object of type 'Socket' cannot be assigned because its copy assignment operator is implicitly deleted` error by defining,

 

Socket(Socket&&)=default; 

Socket& operator=(Socket&&)=default;

The reason that error mentioned Copy assignment op is because if there is no move assignment/constructor present, the copy assignment/copy-constructor must be used instead.

Now, I can't seem get away from the seg fault any sooner. But I notice that the Socket object get destructed according to the print statement.

I store the Socket object in a global unordered map to keep it alive but the output_stream behave so different than other libs I tried. It get  destructed  early all along with the Socket object.

I basically move the output stream to the Socket class's seastar::output_stream<char> socket field.

The application I create need to maintain state and it's not stateless like most networking apps hence the global map.

 So, output_stream  not movable? Do I have to move the "pure" socket object from seastar itself and grab the output stream thereafter? 

On Sunday, September 19, 2021 at 1:09:55 PM UTC-4 Avi Kivity wrote:

[jeffrtc]

What do you mean by I/O has started?

> Passing seastar::socket to your Socket and extracting the output_stream at that point is cleaner IMO 

But the issue with this approach is that I have to close the output_stream and you can't keep it alive.

The map contains a super wrapper class called Connection

Class Connection contains the socket wrapper as a field. 

For example,

class Connection {

   public:

     Socket socket;

};

std::unordered_map<std::string, Connection> connections;

void handle() {

    auto& connection = connections["A"];

    std::cout << "From Handle  " << connection.socket.isLive << "\n";

}

// This is a minimal example not the best complete example b

void worker(Socket&& socket) {

    const auto& [connection, inserted] = connections.try_emplace("A");

    std::cout << "Connection Emplaced" << "\n";

    connection->second.socket = std::move(socket);

    connection->second.socket.isLive = true;

    std::cout << "Socket Moved" << "\n";

    std::async(std::launch::async, handle);

}

I believe emplacing to connections map will cause creation of a Temporary Socket type Object for the Connection Object's socket field.

As you see on my code, I move an actual socket to the the socket field of Connection Object, but that doesn't means that the Temporary Socket Object is gone.  

It's still out there somewhere like Voldemort.  I believe what happens is that the when the worker function goes out of scope the Temporary Socket Object get destructed but it fails with a seg fault.

Below you can see the way I call the worker function on my real world app, 

            seastar::future<> handleConnection(std::string peer, seastar::connected_socket socket) {

                try {

                    while(true) {

                        auto in = socket.input();

                        

                        seastar::data_source::tmp_buf buffer = co_await in.read();

                        if(buffer) {

                            std::string data(buffer.get(), buffer.size());

                            

                            // Create a wrapper socket object with output stream and peer

                            Socket _socket{socket.output(), peer};

                            

                            // The call back has future type, we ignore the the function return and continue the loop

                            (void)messageCallback(peer, data, std::move(_socket));

                        } else {

                            break;

                        }

                    }

                } catch(std::exception_ptr ep) {

                    std::cout << "Connection error" << ep << "\n";

                }

            }

[Avi Kivity]

On 23/09/2021 07.10, jeffrtc wrote:

What do you mean by I/O has started?

A call to output_stream::write() or a similar function.

> Passing seastar::socket to your Socket and extracting the output_stream at that point is cleaner IMO 

But the issue with this approach is that I have to close the output_stream and you can't keep it alive.

The map contains a super wrapper class called Connection

Class Connection contains the socket wrapper as a field. 

For example,

class Connection {

   public:

     Socket socket;

};

std::unordered_map<std::string, Connection> connections;

void handle() {

    auto& connection = connections["A"];

    std::cout << "From Handle  " << connection.socket.isLive << "\n";

}

// This is a minimal example not the best complete example b

void worker(Socket&& socket) {

    const auto& [connection, inserted] = connections.try_emplace("A");

    std::cout << "Connection Emplaced" << "\n";

    connection->second.socket = std::move(socket);

    connection->second.socket.isLive = true;

    std::cout << "Socket Moved" << "\n";

    std::async(std::launch::async, handle);

}

I believe emplacing to connections map will cause creation of a Temporary Socket type Object for the Connection Object's socket field.

Yes, but you could avoid that ny passing socket to connections.try_emplace (and making sure the constructor moves that along). You can look at Seastar's httpd which does the same thing more or less.

    connection(http_server& server, connected_socket&& fd,
            socket_address addr)
            : _server(server), _fd(std::move(fd)), _read_buf(_fd.input()), _write_buf(
                    _fd.output()) {
        on_new_connection();
    }

This connection object lives in an intrusive_list rather than an unordered_map, but the principle is the same.

As you see on my code, I move an actual socket to the the socket field of Connection Object, but that doesn't means that the Temporary Socket Object is gone.  

Even that should just work. As long as the output_stream is not used, it can be moved around.

It's still out there somewhere like Voldemort.  I believe what happens is that the when the worker function goes out of scope the Temporary Socket Object get destructed but it fails with a seg fault.

Below you can see the way I call the worker function on my real world app, 

            seastar::future<> handleConnection(std::string peer, seastar::connected_socket socket) {

                try {

                    while(true) {

                        auto in = socket.input();

                        

                        seastar::data_source::tmp_buf buffer = co_await in.read();

                        if(buffer) {

                            std::string data(buffer.get(), buffer.size());

                            

                            // Create a wrapper socket object with output stream and peer

                            Socket _socket{socket.output(), peer};

                            

                            // The call back has future type, we ignore the the function return and continue the loop

                            (void)messageCallback(peer, data, std::move(_socket));

                        } else {

                            break;

                        }

                    }

                } catch(std::exception_ptr ep) {

                    std::cout << "Connection error" << ep << "\n";

                }

            }

Ignoring returned futures is setting yourself up for a world of pain. There's a reason it's marked nodiscard. Apart from that, I see nothing wrong with the code, maybe the problem is in messageCallback.

To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/b406c64d-255c-4d25-b505-a386cd124fe1n%40googlegroups.com.

[jeffrtc]

I made switch to lw_ptr and yet no change. I also replaced (void) with co_await on messageCallback and yet no change.

I went ahead and build seastar latest under debug mode then compiled program.

Here what I get,

../../include/seastar/core/reactor.hh:752:13: runtime error: reference binding to null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../include/seastar/core/reactor.hh:752:13 in 

../../src/core/reactor.cc:3435:22: runtime error: member call on null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../src/core/reactor.cc:3435:22 in 

../../include/seastar/core/reactor.hh:576:38: runtime error: member access within null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../include/seastar/core/reactor.hh:576:38 in 

AddressSanitizer:DEADLYSIGNAL

This happens as soon as the third party MQ messaging library  invoke it's onPeerMessage callback and attempt to send data using seastar sockets. The third party callback doesn't use Seastar threads.

Maybe this happens because incompatibility between two? 

socket.send works perfectly fine within seastar created thread/future contexts.

[tcha...@gmail.com]

On Monday, September 27, 2021 at 5:08:13 AM UTC+8 jeffrtc wrote:

I made switch to lw_ptr and yet no change. I also replaced (void) with co_await on messageCallback and yet no change.

I went ahead and build seastar latest under debug mode then compiled program.

Here what I get,

../../include/seastar/core/reactor.hh:752:13: runtime error: reference binding to null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../include/seastar/core/reactor.hh:752:13 in 

../../src/core/reactor.cc:3435:22: runtime error: member call on null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../src/core/reactor.cc:3435:22 in 

../../include/seastar/core/reactor.hh:576:38: runtime error: member access within null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../include/seastar/core/reactor.hh:576:38 in 

AddressSanitizer:DEADLYSIGNAL

This happens as soon as the third party MQ messaging library  invoke it's onPeerMessage callback and attempt to send data using seastar sockets. The third party callback doesn't use Seastar threads. 

Maybe this happens because incompatibility between two? 

yup. i think that's exactly the crux of the problem. the engine() is not available on a non-seastar (a.k.a. alien) thread. to allow an alien thread talk to a remote shard, you might want to use seastar::alien::run_on() or seastar::alien::submit_to() [0]

--

[0] http://docs.seastar.io/master/namespaceseastar_1_1alien.html

[jeffrtc]

Wow. This should be documented. I nearly spent week. 

 Is there any example application that uses alien stuff?

[kefu chai]

On Tue, Sep 28, 2021 at 8:57 PM jeffrtc <eugene...@gmail.com> wrote:
>
> Wow. This should be documented. I nearly spent week.
>
> Is there any example application that uses alien stuff?

see https://github.com/scylladb/seastar/blob/master/tests/unit/alien_test.cc

> You received this message because you are subscribed to a topic in the Google Groups "seastar-dev" group.
> To unsubscribe from this topic, visit https://groups.google.com/d/topic/seastar-dev/l3WasEOCrEw/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to seastar-dev...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/fe82ee9b-161c-4c44-9684-1b6b08132079n%40googlegroups.com.



--
Regards
Kefu Chai

[jeffrtc]

It seems to be working and I see the TCP message sent successfully, but still getting pretty much same error afterwards.

../../include/seastar/core/reactor.hh:752:13: runtime error: reference binding to null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../include/seastar/core/reactor.hh:752:13 in 

../../src/core/reactor.cc:3435:22: runtime error: member call on null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../src/core/reactor.cc:3435:22 in 

../../include/seastar/core/reactor.hh:576:38: runtime error: member access within null pointer of type 'struct reactor'

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../include/seastar/core/reactor.hh:576:38 in 

AddressSanitizer:DEADLYSIGNAL

                seastar::future<> send(std::string message) {

                    seastar::alien::submit_to(0, [this, &message] () -> seastar::future<> {

                        co_await socket->write(message);

                        co_return co_await socket->flush();

                    }).wait();

                    co_return;

                }

Is that because I can't  use seastar::future from a foreign thread?  

[Avi Kivity]

The don't-leave-your-own-thread is so ingrained in Seastar developers, we don't even think about it.

It deserves a section in the tutorial, along with an introduction to seastar::alien.

To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/fe82ee9b-161c-4c44-9684-1b6b08132079n%40googlegroups.com.

[jeffrtc]

I made it work by removing seastar::future<> return type.

                void send(std::string message) {

                    seastar::alien::submit_to(0, [this, &message] () -> seastar::future<> {

                        co_await socket->write(message);

                        co_return co_await socket->flush();

                    }).wait();

                }

Avi - I wonder if there is any better way than hard coding shard 1 aka Core 1 in submit_to? I think non-Alien version has some load balancing logic along with some super algorithms I believe.

[Avi Kivity]

Load balancing in Seastar is manual. It's usually data oriented, you direct a message to the shard that owns the data the message relates to.

Typically hash(key) % seastar::smp::count or similar.

To view this discussion on the web visit https://groups.google.com/d/msgid/seastar-dev/57ea349f-2e43-4c67-aae7-a8d6cc72c9ecn%40googlegroups.com.

[jeffrtc]

Example for this hash thingie?