[BUG][FIX] in platform specific code ( [er] changes needed? )

John M McIntosh johnmci at smalltalkconsulting.com
Tue May 11 20:07:48 UTC 2004


Well Ian did a stealth change awhile back.
static int socketValid(SocketPtr s)
{
   if (s && s->privateSocketPtr && thisNetSession && (s->sessionID ==  
thisNetSession))
     return true;
   interpreterProxy->success(false);
   return false;
}

Note the addition of && thisNetSession check which fixes the problem.
versus the older code which was:

static int socketValid(SocketPtr s)
{
   if ((s != 0) && (s->privateSocketPtr != 0) && (s->sessionID ==  
thisNetSession))
     return true;
   interpreterProxy->success(false);
   return false;
}

This is the WIndows code (I think)
/ 
************************************************************************ 
*****
   SocketValid: Validate a given SocketPtr
************************************************************************ 
*****/
static int SocketValid(SocketPtr s) {
   if ((s != NULL) &&
       (s->privateSocketPtr != NULL) &&
       (s->sessionID == thisNetSession)) {
     return true;
   } else {
     FAIL();
     return false;
   }
}

Someone (I sure there is a windows user out there somewhere?) could try  
the suggested failure case (attached below)
and see if this problem happens on the windows side? Since the copy of  
the windows code I have looks like the old faulty Unix code.


On May 11, 2004, at 12:24 PM, tim at sumeru.stanford.edu wrote:

>
> It looks like no one but John has yet made any change relating to this
> problem; should we? It looks a simple enough change but I'm vague  
> enough
> about sockets to not know whether it is sufficient.
>
>
>
Original note

This change prevents the following failure case where you can create a  
socket before initializing the network which
then causes the VM to core dump after a save/restart when it attempts  
to destroy the socket say as a result of weak reference cleanup.

This won't work on the macintosh VMs because another check fails the  
socket creation because OpenTransport isn't initialized yet, and I did  
roll the fix into the macintosh vm 3.5.1b4 because it uses the unix  
socket code now. But you alert Unix/Linux users might want to try this  
and see what happens.

Anyone know what Windows does?

| socket |
	socket _ Socket newTCP.
	socket listenOn: 44444 backlogSize: 4.
	^socket

this gives you

semaphore: 	a Semaphore()
socketHandle: 	a ByteArray(0 0 0 0 0 0 0 0 2 96 51 16)  /*Note how  
session id is ZERO */
readSemaphore: 	a Semaphore()
writeSemaphore: 	a Semaphore()
primitiveOnlySupportsOneSemaphore: 	false

Now save the image, restart, then issue a destroy on that Socket and  
BOOM
{I'll note sometimes I don't get a crash but the debugging malloc code  
complains about an invalid free, or double free since
the privateptr is bogus but read/writeable...}

I got this because  KomServices does this.
TcpListener>>newListener: backlogSize
	"Create a new socket that listens on our port.  The backlog is how  
many simultaneous
	connections to accept at the same time"

	[^self pvtNewListener: backlogSize] on: Error do: [].

	"Try one more time after initializing the network"
	Socket initializeNetwork.
	^self pvtNewListener: backlogSize.

TcpListener>> pvtNewListener: backlogSize
	"Create a new socket that listens on our port.  The backlog is how  
many simultaneous
	connections to accept at the same time"

	| listener |
	listener := self socketClass newTCP.
	self socketsToDestroy add: listener.
	listener listenOn: portNumber backlogSize: backlogSize.
	^listener


I have to wonder if an accept can occur and we can get a socket created  
with a zero session id too?

New unix code should look like below. Can't speak for what the windows  
code should look like, but the key is that thisNetSession should NOT be  
zero when we run this check.

/* answer whether the given socket is valid in this net session */

static int socketValid(SocketPtr s)
{
   if ((s != 0) && (s->privateSocketPtr != 0) && (s->sessionID ==  
thisNetSession) && (thisNetSession != 0))
     return true;
   interpreterProxy->success(false);
   return false;
}

Will need someone to approve and make the unix code changes...
-

--
======================================================================== 
===
John M. McIntosh <johnmci at smalltalkconsulting.com> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
======================================================================== 
===




More information about the Squeak-dev mailing list