A couple of questions:

1) Regarding the memory allocation problem, i wonder if it is advise to use smart pointers, so I don't need to deallocate myself ?

2) When recv() function return -1 or 0, it is correct to call closesocket() or no ?

1. Smart pointers are usually advised in C++.

With that said though, as long as both your client_server and server_client thread functions exit when they are supposed to, you can just keep a mutex object and reference count in SocketPair struct to know when you can delete the pointers. For example, have a look at the mutex example. You just acquire the mutex at the start of the thread, increment a counter, then release the mutex. At the end of the thread function, you acquire the mutex, decrement the counter, check to see if it's 0, if so delete the memory, then release the mutex. Don't forget to correctly instantiate the members though.

2. From MSDN:

An application should always have a matching call to closesocket for each successful call to socket to return any socket resources to the system.[/quote]

So you do need a matching call to closesocket, but where you do that just depends on your application. Since you have two threads each handling the same socket, you'd only want one of them to call it. Also, you want to make sure you handle as graceful shutdown as possible and call shutdown if you are going to be disconnecting the socket yourself.

Also, I noticed another problem in the earlier code you posted. You cannot have a global SocketPair object like you do right now. Since you are using threads, you need to allocate memory for the object before passing it to the threads. The threads would then deallocate the memory as well. This could contribute to some other issues you might be seeing if you were to connect more than one connection through your proxy. So get rid of the global object and allocate one for each pair in your dispatchConnection function.

You will need to check your program for other global variable issues as well. When you use threads, you have to synchronize access for reading/writing between shared variables. If you do not, you get undefined results, of which may result in the program working sometimes, but not others.

If I understood good its something like this ?



class SocketPair {
private:
TCPSocket* clientProxySocket;
ClientSocket* proxyServerSocket;
pthread_mutex_t mutex;
int counter;
public:
SocketPair()
{
mutex = PTHREAD_MUTEX_INITIALIZER;
counter = 0;
}
~SocketPair() {}
void addClientProxy(TCPSocket* clientProxySocket)
{
this->clientProxySocket = clientProxySocket;
}
void addProxyServer(ClientSocket* proxyServerSocket)
{
this->proxyServerSocket = proxyServerSocket;
}
TCPSocket* getClientProxy()
{
return clientProxySocket;
}
ClientSocket* getProxyServer()
{
return proxyServerSocket;
}
pthread_mutex_t getMutex()
{
return mutex;
}
void incr()
{
++counter;
}
void decr()
{
--counter;
}
int getCounter()
{
return counter;
}
};





void* client_server(void* arg)
{
pthread_detach(pthread_self());

char data[65536];
SocketPair* socketPair = (SocketPair*)arg;

pthread_mutex_t mutex = socketPair->getMutex();
pthread_mutex_lock(&mutex);
socketPair->incr();
pthread_mutex_unlock(&mutex);

while(true)
{
// First, receive as many bytes as possible
int bytes_recv = socketPair->getClientProxy()->Recv(data, sizeof(data));

// 0 for disconnect, -1 for error
if( bytes_recv <= 0)
{
cout << "Error receiving bytes from client" << endl;
break;
}

// Now try to send all the bytes
int bytes_sent = socketPair->getProxyServer()->Send(data, bytes_recv);

// Debugging info
cout <<"Received from client: " << bytes_recv << endl;
cout << "Sent to web server: " << bytes_sent << endl;

// On success, bytes_sent should equal bytes_recv
if( bytes_sent != bytes_recv )
{
cout << "Error, didnt send all bytes from client to server" << endl;
break;
}
}

pthread_mutex_lock(&mutex);
socketPair->decr();
if((socketPair->getCounter()) == 0)
{
delete socketPair->getClientProxy();
delete socketPair->getProxyServer();
delete socketPair;
}
pthread_mutex_unlock(&mutex);

return NULL;
}








void dispatchConnection(TCPSocket* clientProxySocket, ClientSocket* proxyServerSocket)
{
Handle handle(clientProxySocket);
handle.HandleAuthentication();

string addr = handle.getAddress();
short prt = handle.getPort();

proxyServerSocket->Connect(addr, prt);

// Create a SocketPair object and populate it
SocketPair* socketPair = new SocketPair;
socketPair->addClientProxy(clientProxySocket);
socketPair->addProxyServer(proxyServerSocket);

// create threads
if(pthread_create(&pid1, NULL, client_server, &socketPair) != 0)
{
cout << "Unable to create pipe client-server thread (pthread_create()" << endl;
}
if(pthread_create(&pid2, NULL, server_client, &socketPair) != 0)
{
cout << "Unable to create pipe server-client thread (pthread_create()" << endl;
}
}




Getting better, you have a few more design flaws.

1. The "pthread_mutex_t getMutex()" function currently returns a copy of the mutex object. You instead want to return a reference of it. You could also just make it return a pointer, but if you do that, you will need to update all your calls to pthread_mutex_lock/unlock so they don't pass the address (see #4 below).

2. You must release the mutex before deleting the class object (socketPair)! Otherwise, you are deleting the mutex and then trying to release it, which is undefined behavior .

3. You can move "pthread_t pid1, pid2;" into the SocketPair class so keep track of the threads that are being used. Right now, your program will only track the last two threads created. Consider just making them public member variables rather than trying to write get/set for them, but it's up to you.

4. Now that the socketPair object is a pointer, you do not need to pass its address in pthread_create; you just pass the pointer address. Right now, you are passing a SocketPair ** to the function, so it should crash when you run it.

5. You can also consider calling the close socket logic before you delete the variables if you do not have it anywhere else right now. That would be the ideal place since you do not really have anywhere else to put it.

Yes i will make all the methods and elements public in SocketPair, is much easy. I knew that #2 will give me that problem, then i will try to move it after i release the mutex. Regarding #4, it doesnt crash, but it doesnt do nothing, just gives me error in receiving data (strerrno = "No Error"),so i knew something its not good. And about #5 i am calling closesocket() in the destructor of Socket() so when i call delete to release memory, i guess it will call closesocket() too, its ok like this ?

Well, I am a litlle shocked....it works ! I open two sites,without problem, but then it crashes I'm glad that I am getting results.

Using backtrace in gdb it gives me this:



Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 7396.0x1d74]
0x6e0c30cf in pthread_mutex_lock () from c:\MinGW\bin\libpthread-2.dll
(gdb) backtrace
#0 0x6e0c30cf in pthread_mutex_lock () from c:\MinGW\bin\libpthread-2.dll
#1 0x00401828 in _fu9___ZSt4cout (arg=0x3647a0) at ..\src\server.cpp:484
#2 0x6e0c5121 in ptw32_threadStart@4 () from c:\MinGW\bin\libpthread-2.dll
#3 0x76cc1287 in msvcrt!_itow_s () from C:\Windows\system32\msvcrt.dll
#4 0x76cc1328 in msvcrt!_endthreadex () from C:\Windows\system32\msvcrt.dll
#5 0x758e1174 in KERNEL32!AcquireSRWLockExclusive ()
from C:\Windows\system32\kernel32.dll
#6 0x771bb3f5 in ntdll!RtlInsertElementGenericTableAvl ()
from C:\Windows\system32\ntdll.dll
#7 0x771bb3c8 in ntdll!RtlInsertElementGenericTableAvl ()
from C:\Windows\system32\ntdll.dll
#8 0x00000000 in ?? ()






EDIT: I'm not sure, but could the problem be when I release memory for socketPair?



{

// more code here.....

pthread_mutex_lock(&socketPair->mutex);
socketPair->counter--;
if(socketPair->counter == 0)
{
delete socketPair->clientProxySocket;
delete socketPair->proxyServerSocket;
}
pthread_mutex_unlock(&socketPair->mutex);
delete socketPair;

return NULL;


}



Yes, thats way i made the mutex global so I can include delete socketPair in the if() test. Now its working for a few sites, but again it crashes this time when the authentication takes place. I will post the code here if I dont find the answer.

Thanks.

I run some tests and i see that are 3 different type of crashes everytime. The crashes are happening, after loading one site or a few sites.

1)



Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 2784.0xd0c]
0x004032c4 in TCPSocket::Send (this=0xfeeefeee,
buffer=0xa56ff04 "HTTP/1.1 200 OK\r\nDate: Fri, 04 Mar 2011 11:15:11 GMT\r\n
Server: Apache\r\nLast-Modified: Fri, 04 Mar 2011 10:46:03 GMT\r\nETag: \"682344
a-11a9-49da5da3650c0\"\r\nAccept-Ranges: bytes\r\nVary: Accept-Encoding,User"...
, size=2231) at ..\src\server.cpp:118
warning: Source file is more recent than executable.
118 int n = ::send(descriptor_m, buffer + offset, si
ze - offset, 0);




2)



Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 3196.0x784]
0x004031f1 in TCPSocket::Recv (this=0xfeeefeee, buffer=0x311ff04 "",
size=65536) at ..\src\server.cpp:134
134 int n = ::recv(descriptor_m, buffer, size, 0);




3)



Program received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 336.0x4e0]
0x773a4685 in ntdll!RtlpSetUserPreferredUILanguages ()
from C:\Windows\system32\ntdll.dll







Anyone knows what could be the problem ?

Looks like you are deleting a variable but still using it somewhere else. After you delete your vars, set them to NULL to see if you can trigger a null pointer crash sooner, rather than later. Make sure you set 'counter' correctly too.

You might have to handle a rare edge case when one thread finishes before the second one starts. In that case, you need another flag variable that is set after counter == 2 on thread start. Before you execute the decrement and then check for 0, you have to wait until that flag is set. Otherwise, one thread might run and delete the class before the second one starts. When the second one starts, the memory is already gone so it crashes. This is just a side effect of using threads and shared memory, you have to guard against such race conditions.

Also make sure you clean and rebuild, one of the warning messages looks like your files are out of sync to the exe.