Import System.Runtime.Serialization from referencesource. #1736
Conversation
It builds on net_4_5 and monodroid so far. We still need to eliminate dynamic code generation to make it work on iOS.
We don't use neither of expression trees nor SRE anyways, but it's easier to reduce noises in nunit test failures to review the rest.
…ion stubs. The new code is to implement reflection (non-emit) based serialization so that it works on iOS. Actually I _totally_ switched to this new implementation because dynamic method based implementation also doesn't work fine, even on desktop (due to invocation failure on internal methods, it seems that our dynamic method needs some bugfixes). The new methods with NotImplementedException in new classes need to be implemented.
Though it's still buggy and shows several nunit test failures yet.
It was inconsistent with what referencesource codegen does.
Now it should match referencesource behavior.
…ary. Collection serialization was mostly failing, when it was non-dictionary. Now it should be closer to referencesource dynamic code behavior.
Array elements were not deserialized correctly and causing null (default) items put into the array.
… interpreter. Basically the primary cause of the problem was that invoked primitive array reader methods via reflection had 'out' parameter, which needed special care.
…ce codegen.
The original code is complicated enough that I misinterpreted the logic...
The reulting code should look like this:
DateTime? expr_94 = timeSubmitted;
DateTime dateTime;
bool arg_B5_0;
if (XmlObjectSerializerWriteContext.GetHasValue<DateTime> (expr_94)) {
dateTime = XmlObjectSerializerWriteContext.GetNullableValue<DateTime> (expr_94);
arg_B5_0 = false;
}
else {
dateTime = XmlObjectSerializerWriteContext.GetDefaultValue<DateTime> ();
arg_B5_0 = true;
}
The fixed code should be like this now.
The removed code caused unexpected omission of xsi:type by giving "false" Type information that is from the actual value (when the expected type is different, xsi:type should be written).
…n is also needed.
There is no BOM output on Windows either.
…havior. It is likely that the second contract conflicts with the existing one and WCF renames it to avoid that.
WriteStartDocument() can be used with document and document does not accept more than one element, which basically contradicts MTOM writer usage. For EOL changes, MTOM writer should not alter content EOL chars.
They don't pass all the tests in System.ServiceModel.Web yet.
…ep it open. Otherwise MemoryStream.Position complains that it is already disposed.
Unlike our own XsdDataContractExporter, referencesource one generates an extra schema that targets xs:* (XmlSchema.Namespace). That should not be generated as part of WSDL, so exclude it.
…xmlns-es. The namespace declarations existed when the entire Message is read but not in the partial body contents. They caused regressions when we use serialization stack from referencesource. It is possible that more namespaces may be requied, but we will be importing System.ServiceModel later on and then the issue will go away. So far we don't want regression as long as they show up in our NUnit tests.
…re]d. .NET WCF JSON serializer has been known to be buggy...
…ET IN JST. Those tests were added without considering TimeZone difference. They cause several kind of errors (SerializationException etc.) in JST (+09:00) or any timezone that is earlier than UTC. We likely had some "working" implementation, but was incompatible with .NET.
…sibility. I wonder why, but when switched to referencesource, it turned out that ResourceCollectionInfo calls SyndicationElementExtensionCollection.Add() which targets a private overload, but (of course) failed and resolved to another overload that takes object, and thus resulting in invalid code path. With this fix the call to Add() resolves to the expected one and does not regress with referencesource import.
JSON array is deserialized as Array, not List<T>.
Just like BinaryMessageFormatter needed to use another CreateBinaryWriter() overload to NOT close the input stream, use CreateTextWriter() overload that does not close the stream.
WS-Discovery never got working, skip cosmetic test to not block porting.
Import System.Runtime.Serialization from referencesource.
|
Hey @atsushieno, I just traced back a crash I'm seeing in the System.Runtime.Serialization testsuite to this PR. It crashes consistently with a SIGSEGV after showing the test results on Linux (amd64/Ubuntu 14.04): Is this something you're seeing on your end as well? |
|
I see it as well. That's a runtime issue that the runtime team has to fix. |
|
Can we revert this in the meanwhile? |
|
Do you really understand that you are going to revert EVERYTHING? You should consult @migueldeicaza on that. |
|
There's will be plenty left if we revert this. |
|
Let me rephrase: you should consult @migueldeicaza. It is HUGE revert. |
|
I fail to see the point of consulting him on such a simple technical decision as I'd be asking "Can Atsushi and I discuss and make a decision on System.Runtime.Serialization?". We're empowered to make this sort of decision, so let's do it! What I'm trying to figure out is if it can be reverted until we fix the issue and then reapplied. That's all, I'm asking for your technical assessment on it. Since we have a merge commit, reverting/reapplying this as whole would, in theory, be easy. Would a revert of this PR fix the crash? IOW, would it leave us with a mono as good as what If reverting turns to be complicated let's keep it in. But if it's super easy I think it's worth considering temporarily doing it. |
|
I think the problem here is the crash, which will still be in the runtime even if we revert this, it just won't appear. |
|
@akoeplinger do you think you could get repro for the crash? |
|
@marek-safar just run the testsuite on Linux, it is 100% consistent (and @atsushieno confirmed he sees it as well on his end). |
|
There are several reasons I find reverting this is stupid:
The negative impact of reverting this change is that there are not a few Xamarin customers who are looking for the referencesource merge and we kept telling them that it will be fixed in the next major release. If there is important reason that we have to revert it, then they would find it reasonable. The reason you want to revert this is just for nominal green build, which won't persuade Xamarin customers. |
|
#1779 seems to fix this. |
|
Hey guys, I take it there was no need to revert this since #1779 was applied? |
|
#1779 has not been merged, that PR was still under work last time I checked. |
This time it is really ready to be imported.