30914 - PR_PushIOLayer should not swap the contents of the fd's

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[Wan-Teh Chang]

When we push a fd onto the top of a fd stack,
the current implementation of PR_PushIOLayer
swaps the contents of the new fd and the fd at
the top of the stack so that the address of
the top of the stack won't change. (PR_PopIOLayer
switches the contents back if the top of a stack
is popped.)  While this fd swapping is not obvious,
things do work correctly if you push and pop the
fd's in the right order.

However, this fd swapping is problematic if you
pass a pointer to a fd to some callback function
and then push some other fd on top of that fd.
The pointer you passed to the callback function
will be pointing to the contents of the new
fd.  We ran into this problem in our Mac socket
I/O code where we pass a pointer to a fd to OpenTransport
notifier routines.

We can avoid the fd content swapping by adding a dummy
top fd and always push new fd's underneath the
dummy top fd.  Then the address of the top of the stack
will also remain unchanged after pushing and popping.

Since this change will break backward compatibility,
it can only be considered for NSPR 5.0.

Comment 1

[srinivas]

How about adding the new functionality without breaking existing API:

- Define new functions, say, PR_CreateLayer and PR_DestroyLayer, to create and 
destroy the dummy FD.
- Modify PR_PushIOLayer/PopIOLayer to recognize the dummy FD (at the top of the 
stack). These functions work the same way for the current type of layers.

Comment 2

[Wan-Teh Chang]

That's a good idea.  We probably can get by with just
a PR_CreateLayer function.  It seems that the functionality
of PR_DestroyLayer can be subsumed by PR_PopIOLayer or
PR_Close.

Comment 3

[srinivas]

The following patch defines a new function, PR_CreateIOLayer, for a creating the 
new style stack with a dummy header. To destroy a stack, PR_Close can be called 
for the top of the stack.
The test program pr/tests/layer.c is modified to create the new style stack.

Comment 4

[srinivas]

Attachment: Patch for the new style stack.

Comment 5

[srinivas]

Checked in the patch.

Files modified:

/cvsroot/mozilla/nsprpub/pr/include/prio.h,v  <--  prio.h
new revision: 3.22; previous revision: 3.21

/cvsroot/mozilla/nsprpub/pr/src/io/prlayer.c,v  <--  prlayer.c
new revision: 3.11; previous revision: 3.10

/cvsroot/mozilla/nsprpub/pr/tests/layer.c,v  <--  layer.c
new revision: 3.7; previous revision: 3.6

Comment 6

[srinivas]

Markign fixed.