OpenRTMFP / Cumulus

CumulusServer is a complete open source and cross-platform RTMFP server extensible by way of scripting
http://groups.google.com/group/openrtmfp-cumulus
GNU General Public License v3.0
593 stars 221 forks source link

are FlowWriter's operations non-thread-safe? #73

Closed Poechant closed 12 years ago

Poechant commented 12 years ago

is there a lock for sending messages into and fetching messages from the message queue? I guess this message queue is non-thread-safe.

:)

Poechant

cumulusdev commented 12 years ago

where is this lock? (file, line)

Poechant commented 12 years ago

Oh, I mean perhaps there should be such a lock. the messages in FlowWriter has two important operations: pop_front and push_back. When invoking:

    AMFWriter& FlowWriter::writeAMFMessage(const std::string& name) {
        MessageBuffered& message(createBufferedMessage());
        writeResponseHeader(message.rawWriter,name,0);
        return message.amfWriter;
    }

    MessageBuffered& FlowWriter::createBufferedMessage() {
        if(_closed || signature.empty() || _band.failed()) // signature.empty() means that we are on the flowWriter of FlowNull
            return _MessageNull;
        MessageBuffered* pMessage = new MessageBuffered();
        _messages.push_back(pMessage);
        return *pMessage;
    }

data is pushed into the message queue. But the pointer to the message has been returned. Then if

    AMFWriter& amf = client.writer().writeAMFMessage("someMsg");
    amf.writeNumber(abc);
    amf.writeNumber(def);
    amf.writeNumber(ghi);

After the message was pushed into the queue, data could still be writes into the message. And pop operations may be called in some other places. This will cause a problem that after the message has been popped the operation of writing something into the message.

I think the "push message into the queue" operation and "write data into the message" operation should be atomic. Thus, a ScopedMutex may be necessary.

:)

cumulusdev commented 12 years ago

No, don't worry, it's thread safe. Cumulus in the current online version is very stable :-) it's used in production ;-)

Poechant commented 12 years ago

Thanks for your reply :)

I think CumulusLib is very very helpful for developing other applications using RTMFP. Considering such applications, perhaps adding mutex and lock is worthy thinking twice :)

cumulusdev commented 12 years ago

beware, you can link CumulusLib just inside a GPL software.