VM: [Kernel] Revert changes to the native ports that introduced peers.

The problem with these changes was that closed ports still continue to receive
messages and as a result we start using dead peer objects.

Fixing that would require more intrusive changes into message handler implementation - so instead we are reverting the changes and restoring manual PORT -> PEER mapping.

BUG=
R=erikcorry@google.com

Review-Url: https://codereview.chromium.org/2666063002 .
This commit is contained in:
Vyacheslav Egorov 2017-01-31 10:30:15 +01:00
parent ef31637857
commit 9bae50bf47
11 changed files with 91 additions and 79 deletions

View file

@ -27,9 +27,7 @@ namespace bin {
response = type::method##Request(data); \
break;
void IOServiceCallback(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
void IOServiceCallback(Dart_Port dest_port_id, Dart_CObject* message) {
Dart_Port reply_port_id = ILLEGAL_PORT;
CObject* response = CObject::IllegalArgumentError();
CObjectArray request(message);
@ -57,7 +55,7 @@ void IOServiceCallback(Dart_Port dest_port_id,
Dart_Port IOService::GetServicePort() {
return Dart_NewNativePort("IOService", IOServiceCallback, true, NULL);
return Dart_NewNativePort("IOService", IOServiceCallback, true);
}

View file

@ -26,9 +26,7 @@ namespace bin {
response = type::method##Request(data); \
break;
void IOServiceCallback(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
void IOServiceCallback(Dart_Port dest_port_id, Dart_CObject* message) {
Dart_Port reply_port_id = ILLEGAL_PORT;
CObject* response = CObject::IllegalArgumentError();
CObjectArray request(message);
@ -56,7 +54,7 @@ void IOServiceCallback(Dart_Port dest_port_id,
Dart_Port IOService::GetServicePort() {
return Dart_NewNativePort("IOService", IOServiceCallback, true, NULL);
return Dart_NewNativePort("IOService", IOServiceCallback, true);
}

View file

@ -34,8 +34,7 @@ Loader::Loader(IsolateData* isolate_data)
payload_length_(0) {
monitor_ = new Monitor();
ASSERT(isolate_data_ != NULL);
port_ =
Dart_NewNativePort("Loader", Loader::NativeMessageHandler, false, this);
port_ = Dart_NewNativePort("Loader", Loader::NativeMessageHandler, false);
isolate_data_->set_loader(this);
AddLoader(port_, isolate_data_);
}
@ -818,10 +817,13 @@ Loader* Loader::LoaderFor(Dart_Port port) {
void Loader::NativeMessageHandler(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
Dart_CObject* message) {
MutexLocker ml(loader_infos_lock_);
static_cast<Loader*>(peer)->QueueMessage(message);
Loader* loader = LoaderForLocked(dest_port_id);
if (loader == NULL) {
return;
}
loader->QueueMessage(message);
}
} // namespace bin

View file

@ -142,8 +142,7 @@ class Loader {
// This is the global callback for the native message handlers.
static void NativeMessageHandler(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer);
Dart_CObject* message);
};
} // namespace bin

View file

@ -124,8 +124,7 @@ DART_EXPORT bool Dart_PostInteger(Dart_Port port_id, int64_t message);
*/
typedef void (*Dart_NativeMessageHandler)(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer);
Dart_CObject* message);
/**
* Creates a new native port. When messages are received on this
@ -136,16 +135,13 @@ typedef void (*Dart_NativeMessageHandler)(Dart_Port dest_port_id,
* \param handler The C handler to run when messages arrive on the port.
* \param handle_concurrently Is it okay to process requests on this
* native port concurrently?
* \param peer Peer associated with this port. It will be passed down to the
handler when new message arrives.
*
* \return If successful, returns the port id for the native port. In
* case of error, returns ILLEGAL_PORT.
*/
DART_EXPORT Dart_Port Dart_NewNativePort(const char* name,
Dart_NativeMessageHandler handler,
bool handle_concurrently,
void* peer);
bool handle_concurrently);
/* TODO(turnidge): Currently handle_concurrently is ignored. */
/**

View file

@ -7150,19 +7150,12 @@ TEST_CASE(ImportLibrary5) {
}
void NewNativePort_send123(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
void NewNativePort_send123(Dart_Port dest_port_id, Dart_CObject* message) {
// Gets a send port message.
EXPECT_NOTNULL(message);
EXPECT_EQ(Dart_CObject_kArray, message->type);
EXPECT_EQ(Dart_CObject_kSendPort, message->value.as_array.values[0]->type);
// Check peer validity.
EXPECT_NOTNULL(peer);
EXPECT_EQ(static_cast<intptr_t>(0xDEADBEEF), *static_cast<intptr_t*>(peer));
*static_cast<intptr_t*>(peer) = 123;
// Post integer value.
Dart_CObject* response =
reinterpret_cast<Dart_CObject*>(Dart_ScopeAllocate(sizeof(Dart_CObject)));
@ -7173,19 +7166,12 @@ void NewNativePort_send123(Dart_Port dest_port_id,
}
void NewNativePort_send321(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
void NewNativePort_send321(Dart_Port dest_port_id, Dart_CObject* message) {
// Gets a null message.
EXPECT_NOTNULL(message);
EXPECT_EQ(Dart_CObject_kArray, message->type);
EXPECT_EQ(Dart_CObject_kSendPort, message->value.as_array.values[0]->type);
// Check peer validity.
EXPECT_NOTNULL(peer);
EXPECT_EQ(static_cast<intptr_t>(0xDEADBEEF), *static_cast<intptr_t*>(peer));
*static_cast<intptr_t*>(peer) = 321;
// Post integer value.
Dart_CObject* response =
reinterpret_cast<Dart_CObject*>(Dart_ScopeAllocate(sizeof(Dart_CObject)));
@ -7212,13 +7198,12 @@ TEST_CASE(IllegalPost) {
UNIT_TEST_CASE(NewNativePort) {
// Create a port with a bogus handler.
Dart_Port error_port = Dart_NewNativePort("Foo", NULL, true, NULL);
Dart_Port error_port = Dart_NewNativePort("Foo", NULL, true);
EXPECT_EQ(ILLEGAL_PORT, error_port);
intptr_t peer1 = 0xDEADBEEF;
// Create the port w/o a current isolate, just to make sure that works.
Dart_Port port_id1 =
Dart_NewNativePort("Port123", NewNativePort_send123, true, &peer1);
Dart_NewNativePort("Port123", NewNativePort_send123, true);
TestIsolateScope __test_isolate__;
const char* kScriptChars =
@ -7236,9 +7221,8 @@ UNIT_TEST_CASE(NewNativePort) {
Dart_EnterScope();
// Create a port w/ a current isolate, to make sure that works too.
intptr_t peer2 = 0xDEADBEEF;
Dart_Port port_id2 =
Dart_NewNativePort("Port321", NewNativePort_send321, true, &peer2);
Dart_NewNativePort("Port321", NewNativePort_send321, true);
Dart_Handle send_port1 = Dart_NewSendPort(port_id1);
EXPECT_VALID(send_port1);
@ -7254,7 +7238,6 @@ UNIT_TEST_CASE(NewNativePort) {
EXPECT(Dart_IsError(result));
EXPECT(Dart_ErrorHasException(result));
EXPECT_SUBSTRING("Exception: 123\n", Dart_GetError(result));
EXPECT_EQ(123, peer1);
// result second port.
dart_args[0] = send_port2;
@ -7264,7 +7247,6 @@ UNIT_TEST_CASE(NewNativePort) {
EXPECT(Dart_IsError(result));
EXPECT(Dart_ErrorHasException(result));
EXPECT_SUBSTRING("Exception: 321\n", Dart_GetError(result));
EXPECT_EQ(321, peer2);
Dart_ExitScope();
@ -7275,8 +7257,7 @@ UNIT_TEST_CASE(NewNativePort) {
void NewNativePort_sendInteger123(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
Dart_CObject* message) {
// Gets a send port message.
EXPECT_NOTNULL(message);
EXPECT_EQ(Dart_CObject_kArray, message->type);
@ -7289,8 +7270,7 @@ void NewNativePort_sendInteger123(Dart_Port dest_port_id,
void NewNativePort_sendInteger321(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
Dart_CObject* message) {
// Gets a null message.
EXPECT_NOTNULL(message);
EXPECT_EQ(Dart_CObject_kArray, message->type);
@ -7318,9 +7298,9 @@ TEST_CASE(NativePortPostInteger) {
Dart_EnterScope();
Dart_Port port_id1 =
Dart_NewNativePort("Port123", NewNativePort_sendInteger123, true, NULL);
Dart_NewNativePort("Port123", NewNativePort_sendInteger123, true);
Dart_Port port_id2 =
Dart_NewNativePort("Port321", NewNativePort_sendInteger321, true, NULL);
Dart_NewNativePort("Port321", NewNativePort_sendInteger321, true);
Dart_Handle send_port1 = Dart_NewSendPort(port_id1);
EXPECT_VALID(send_port1);
@ -7355,8 +7335,7 @@ TEST_CASE(NativePortPostInteger) {
void NewNativePort_nativeReceiveNull(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
Dart_CObject* message) {
EXPECT_NOTNULL(message);
if ((message->type == Dart_CObject_kArray) &&
@ -7386,8 +7365,8 @@ TEST_CASE(NativePortReceiveNull) {
Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL);
Dart_EnterScope();
Dart_Port port_id1 = Dart_NewNativePort(
"PortNull", NewNativePort_nativeReceiveNull, true, NULL);
Dart_Port port_id1 =
Dart_NewNativePort("PortNull", NewNativePort_nativeReceiveNull, true);
Dart_Handle send_port1 = Dart_NewSendPort(port_id1);
EXPECT_VALID(send_port1);
@ -7409,8 +7388,7 @@ TEST_CASE(NativePortReceiveNull) {
void NewNativePort_nativeReceiveInteger(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
Dart_CObject* message) {
EXPECT_NOTNULL(message);
if ((message->type == Dart_CObject_kArray) &&
@ -7441,8 +7419,8 @@ TEST_CASE(NativePortReceiveInteger) {
Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL);
Dart_EnterScope();
Dart_Port port_id1 = Dart_NewNativePort(
"PortNull", NewNativePort_nativeReceiveInteger, true, NULL);
Dart_Port port_id1 =
Dart_NewNativePort("PortNull", NewNativePort_nativeReceiveInteger, true);
Dart_Handle send_port1 = Dart_NewSendPort(port_id1);
EXPECT_VALID(send_port1);

View file

@ -277,8 +277,11 @@ class KernelCompilationRequest : public ValueObject {
: monitor_(new Monitor()),
port_(Dart_NewNativePort("kernel-compilation-port",
&HandleResponse,
false,
this)) {
false)),
next_(NULL),
prev_(NULL) {
ASSERT(port_ != ILLEGAL_PORT);
RegisterRequest(this);
result_.status = Dart_KernelCompilationStatus_Unknown;
result_.error = NULL;
result_.kernel = NULL;
@ -286,6 +289,7 @@ class KernelCompilationRequest : public ValueObject {
}
~KernelCompilationRequest() {
UnregisterRequest(this);
Dart_CloseNativePort(port_);
delete monitor_;
}
@ -364,18 +368,62 @@ class KernelCompilationRequest : public ValueObject {
ml.Notify();
}
static void HandleResponse(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
static_cast<KernelCompilationRequest*>(peer)->HandleResponseImpl(message);
static void HandleResponse(Dart_Port port, Dart_CObject* message) {
MonitorLocker locker(requests_monitor_);
KernelCompilationRequest* rq = FindRequestLocked(port);
if (rq == NULL) {
return;
}
rq->HandleResponseImpl(message);
}
static void RegisterRequest(KernelCompilationRequest* rq) {
MonitorLocker locker(requests_monitor_);
rq->next_ = requests_;
requests_ = rq;
}
static void UnregisterRequest(KernelCompilationRequest* rq) {
MonitorLocker locker(requests_monitor_);
if (rq->next_ != NULL) {
rq->next_->prev_ = rq->prev_;
}
if (rq->prev_ != NULL) {
rq->prev_->next_ = rq->next_;
} else {
requests_ = rq->next_;
}
}
// Note: Caller must hold requests_monitor_.
static KernelCompilationRequest* FindRequestLocked(Dart_Port port) {
for (KernelCompilationRequest* rq = requests_; rq != NULL; rq = rq->next_) {
if (rq->port_ == port) {
return rq;
}
}
return NULL;
}
// This monitor must be held whenever linked list of requests is accessed.
static Monitor* requests_monitor_;
// Linked list of all active requests. Used to find a request by port number.
// Guarded by requests_monitor_ lock.
static KernelCompilationRequest* requests_;
Monitor* monitor_;
Dart_Port port_;
// Linked list of active requests. Guarded by requests_monitor_ lock.
KernelCompilationRequest* next_;
KernelCompilationRequest* prev_;
Dart_KernelCompilationResult result_;
};
Monitor* KernelCompilationRequest::requests_monitor_ = new Monitor();
KernelCompilationRequest* KernelCompilationRequest::requests_ = NULL;
Dart_KernelCompilationResult KernelIsolate::CompileToKernel(
const char* script_uri) {

View file

@ -82,8 +82,7 @@ DART_EXPORT bool Dart_PostInteger(Dart_Port port_id, int64_t message) {
DART_EXPORT Dart_Port Dart_NewNativePort(const char* name,
Dart_NativeMessageHandler handler,
bool handle_concurrently,
void* peer) {
bool handle_concurrently) {
if (name == NULL) {
name = "<UnnamedNativePort>";
}
@ -95,7 +94,7 @@ DART_EXPORT Dart_Port Dart_NewNativePort(const char* name,
// Start the native port without a current isolate.
IsolateSaver saver(Isolate::Current());
NativeMessageHandler* nmh = new NativeMessageHandler(name, handler, peer);
NativeMessageHandler* nmh = new NativeMessageHandler(name, handler);
Dart_Port port_id = PortMap::CreatePort(nmh);
PortMap::SetPortState(port_id, PortMap::kLivePort);
nmh->Run(Dart::thread_pool(), NULL, NULL, 0);

View file

@ -12,9 +12,8 @@
namespace dart {
NativeMessageHandler::NativeMessageHandler(const char* name,
Dart_NativeMessageHandler func,
void* peer)
: name_(strdup(name)), func_(func), peer_(peer) {}
Dart_NativeMessageHandler func)
: name_(strdup(name)), func_(func) {}
NativeMessageHandler::~NativeMessageHandler() {
@ -42,7 +41,7 @@ MessageHandler::MessageStatus NativeMessageHandler::HandleMessage(
Dart_CObject* object;
ApiMessageReader reader(message);
object = reader.ReadMessage();
(*func())(message->dest_port(), object, peer());
(*func())(message->dest_port(), object);
delete message;
return kOK;
}

View file

@ -15,14 +15,11 @@ namespace dart {
// native C handlers.
class NativeMessageHandler : public MessageHandler {
public:
NativeMessageHandler(const char* name,
Dart_NativeMessageHandler func,
void* peer);
NativeMessageHandler(const char* name, Dart_NativeMessageHandler func);
~NativeMessageHandler();
const char* name() const { return name_; }
Dart_NativeMessageHandler func() const { return func_; }
void* peer() const { return peer_; }
MessageStatus HandleMessage(Message* message);
@ -37,7 +34,6 @@ class NativeMessageHandler : public MessageHandler {
private:
char* name_;
Dart_NativeMessageHandler func_;
void* peer_;
};
} // namespace dart

View file

@ -80,8 +80,7 @@ uint8_t* randomArray(int seed, int length) {
void wrappedRandomArray(Dart_Port dest_port_id,
Dart_CObject* message,
void* peer) {
Dart_CObject* message) {
Dart_Port reply_port_id = ILLEGAL_PORT;
if (message->type == Dart_CObject_kArray &&
3 == message->value.as_array.length) {
@ -121,7 +120,7 @@ void randomArrayServicePort(Dart_NativeArguments arguments) {
Dart_EnterScope();
Dart_SetReturnValue(arguments, Dart_Null());
Dart_Port service_port =
Dart_NewNativePort("RandomArrayService", wrappedRandomArray, true, NULL);
Dart_NewNativePort("RandomArrayService", wrappedRandomArray, true);
if (service_port != ILLEGAL_PORT) {
Dart_Handle send_port = HandleError(Dart_NewSendPort(service_port));
Dart_SetReturnValue(arguments, send_port);