Wrong Implementation of single instance application

[AyushExel204]

I have developed a media player which is a single instance application. I have implemented it using QLocalSocket and QLocalServer . So, when another media file is opened using the application, it gets added in the playlist of the already running instance of the application. So, the app works fine when I open different media files one by one but when I select multiple media files and press "Enter", only few of them are added to the playlist and the rest are discarded.
Below is the part of the program which makes it single instance :
Singleinstance.h

#ifndef SINGLEINSTANCE_H
#define SINGLEINSTANCE_H

#include <QObject>
#include <QDebug>
#include <QLocalServer>
#include <QLocalSocket>
#include <QMediaPlaylist>
class SingleInstance : public QObject
{
    Q_OBJECT
public:
     SingleInstance(QObject *parent = 0, QMediaPlaylist* pl=new QMediaPlaylist);
    ~SingleInstance();

    /**
     * @brief Listen for connections
     * @param name
     */
    void listen(QString name);

    /**
     * @brief Determines if a previous instance exists
     * @param name
     * @param arg
     * @return
     */
    bool hasPrevious(QString name, QString arg);

signals:

    /**
     * @brief Emitted when a new instance is launched
     */
    void newInstance();


public slots:

    /**
     * @brief Called when a new instance is opened
     */
    void newConnection();

    /**
     * @brief Read from the socket
     */
    void readyRead();

private:
    /**
     * @brief Local socket
     */
    QLocalSocket* mSocket;

    /**
     * @brief The server
     */
    QLocalServer mServer;
// Pointer to the playlist of mediaPlayer
    QMediaPlaylist* playlist;
};

#endif // SINGLEINSTANCE_H

singleinstace.cpp

#include "singleinstance.h"
SingleInstance::SingleInstance(QObject *parent, QMediaPlaylist *pl) : QObject(parent),playlist(pl)
{
    connect(&mServer, SIGNAL(newConnection()),this,SLOT(newConnection()));
}

SingleInstance::~SingleInstance()
{

mSocket->close();
}

void SingleInstance::listen(QString name)
{
    mServer.removeServer(name);
    mServer.listen(name);


}

bool SingleInstance::hasPrevious(QString name, QString arg)
{


    QLocalSocket socket;
    socket.connectToServer(name, QLocalSocket::ReadWrite);

    if(socket.waitForConnected())
    {
       if(!arg.isEmpty()){ QByteArray buffer;

       buffer.append(arg);
        socket.write(buffer);
        socket.waitForBytesWritten();
       }

        return true;
    }


    return false;

}

void SingleInstance::newConnection()
{
    emit newInstance();

   mSocket = mServer.nextPendingConnection();
   connect(mSocket,SIGNAL(readyRead()),this,SLOT(readyRead()));
}

void SingleInstance::readyRead()
{  //Adding media to the playlist
    playlist->addMedia(QUrl::fromLocalFile(QString::fromStdString(mSocket->readAll().toStdString())));

    playlist->setCurrentIndex(playlist->mediaCount()-1);
    mSocket->deleteLater();

}

How can I fix this code ? Or is there easier way to accomplish this task ?

[VRonin]

in readyRead could you check that mSocket->bytesAvailable() is the same as buffer.size() in hasPrevious?

[AyushExel204]

@VRonin yes they are same

[VRonin]

What's the output of mSocket->readAll() ? could you paste it here?

P.S.

playlist->addMedia(QUrl::fromLocalFile(QString(mSocket->readAll())); // no need to use STD string

SingleInstance(QObject *parent = 0, QMediaPlaylist* pl=new QMediaPlaylist /*<- This is horrible and 99% memory leak guaranteed*/);

[AyushExel204]

@VRonin Okay so I selected the 10 audio files and pressed "Enter" and printed the output of mSocket->readAll() . The output is :

D:\Music\songs\5 second of summer\5 Seconds Of Summer\08 - End Up Here - (www.SongsLover.pk).mp3
D:\Music\songs\5 second of summer\5 Seconds Of Summer\12 - Amnesia - (www.SongsLover.pk).mp3







D:\Music\songs\5 second of summer\5 Seconds Of Summer\Voodoo Doll.mp3

So it is clear that only 3 files are added in the playlist and the address of other files are just replaced by a new line.

[VRonin]

I suspect it has to do with encoding

try replacing buffer.append(arg); with

QDataStream out(&buffer,QIODevice::WriteOnly);
out << arg;

and replace playlist->addMedia(QUrl::fromLocalFile(QString::fromStdString(mSocket->readAll().toStdString()))); with

const QByteArray buffer=mSocket->readAll();
QDataStream in(buffer);
QString argString;
in >> argString;
playlist->addMedia(QUrl::fromLocalFile(argString);

and check that argString has all the lines in it

[AyushExel204]

@VRonin Okay so now even when I open a single media file , it is not added in the playlist . So, I have reverted back to the original code. I want to add that, sometimes the program works correctly and all media files that I select are added in the playlist but most of the times few of them are not added , instead just a blank line is added in the playlist.

[kshegunov]

@AyushExel204

You're overwriting the socket pointer when multiple connections are pending

mSocket = mServer.nextPendingConnection();
connect(mSocket,SIGNAL(readyRead()),this,SLOT(readyRead()));

Then you read from incorrect socket(s):

playlist->addMedia(QUrl::fromLocalFile(QString::fromStdString(mSocket->readAll().toStdString())));

Try this:

void SingleInstance::readyRead()
{ 
    QLocalSocket * socket = qobject_cast<QLocalSocket *>(sender());
    playlist->addMedia(QUrl::fromLocalFile(socket->readAll()));
    // ...
    socket->deleteLater();
}

Also I don't understand why you insist on converting QString to std::string and then back again??!

[VRonin]

I still think that if the file path contains non-ASCII this implementation will fail as it converts from QString to QByteArray with no checks

[kshegunov]

@VRonin

I still think that if the file path contains non-ASCII this implementation will fail as it converts from QString to QByteArray with no checks

Yes, it's quite possible.
Actually it turns out the QString is implicitly converted to utf8 and then added to the byte array. So it might actually work.

[AyushExel204]

@kshegunov said:

QLocalSocket * socket = qobject_cast<QLocalSocket *>(sender());
playlist->addMedia(QUrl::fromLocalFile(socket->readAll()));

Thanks .. this fixed my problem . but can you explain what this code does ?

[jsulm]

sender() returns the pointer to the object which emitted the signal (http://doc.qt.io/qt-5.6/qobject.html#sender).
In this case it is a pointer to QLocalSocket, so you need to cast it from QObject* to QLocalSocket* using qobject_cast